When using explicit Clang modules, some declarations might unexpectedly become invisible.
This is caused by the mechanism that loads PCM files passed via `-fmodule-file=<path>` and creates an `IdentifierInfo` for the module name. The `IdentifierInfo` creation takes place when the `ASTReader` is in a weird state, with modules that are loaded but not yet set up properly. This patch delays the creation of `IdentifierInfo` until the `ASTReader` is done with reading the PCM.
Note that the `-fmodule-file=<name>=<path>` form of the argument doesn't suffer from this issue, since it doesn't create `IdentifierInfo` for the module name.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D111543
During explicit modular build, PCM files are typically specified via the `-fmodule-file=<path>` command-line option. Early during the compilation, Clang uses the `ASTReader` to read their contents and caches the result so that the module isn't loaded implicitly later on. A listener is attached to the `ASTReader` to collect names of the modules read from the PCM files. However, if the PCM has already been loaded previously via PCH:
1. the `ASTReader` doesn't do anything for the second time,
2. the listener is not invoked at all,
3. the module load result is not cached,
4. the compilation fails when attempting to load the module implicitly later on.
This patch solves this problem by attaching the listener to the `ASTReader` for PCH reading as well.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D111560
This patch propagates the import `SourceLocation` into `HeaderSearch::lookupModule`. This enables remarks on search path usage (implemented in D102923) to point to the source code that initiated header search.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D111557
Add -cc1 flags `-fmodules-uses-lock` and `-fno-modules-uses-lock` to
allow the lock manager to be turned off when building implicit modules.
Add `-Rmodule-lock` so that we can see when it's being used.
Differential Revision: https://reviews.llvm.org/D95583
This renames `compileModuleAndReadAST`, adding a `BehindLock` suffix,
and refactors it to significantly reduce nesting.
- Split out helpers `compileModuleAndReadASTImpl` and
`readASTAfterCompileModule` which have straight-line code that doesn't
worry about locks.
- Use `break` in the interesting cases of `switch` statements to reduce
nesting.
- Use early `return`s to reduce nesting.
Detangling the compile-and-read logic from the check-for-locks logic
should be a net win for readability, although I also have a side
motivation of making the locks optional in a follow-up.
No functionality change here.
Differential Revision: https://reviews.llvm.org/D95581
It was possible to re-add a module to a shared in-memory module cache
when search paths are changed. This can eventually cause a crash if the
original module is referenced after this occurs.
1. Module A depends on B
2. B exists in two paths C and D
3. First run only has C on the search path, finds A and B and loads
them
4. Second run adds D to the front of the search path. A is loaded and
contains a reference to the already compiled module from C. But
searching finds the module from D instead, causing a mismatch
5. B and the modules that depend on it are considered out of date and
thus rebuilt
6. The recompiled module A is added to the in-memory cache, freeing
the previously inserted one
This can never occur from a regular clang process, but is very easy to
do through the API - whether through the use of a shared case or just
running multiple compilations from a single `CompilerInstance`. Update
the compilation to return early if a module is already finalized so that
the pre-condition in the in-memory module cache holds.
Resolves rdar://78180255
Differential Revision: https://reviews.llvm.org/D105328
Original commit message:
[clang-repl] Implement partial translation units and error recovery.
https://reviews.llvm.org/D96033 contained a discussion regarding efficient
modeling of error recovery. @rjmccall has outlined the key ideas:
Conceptually, we can split the translation unit into a sequence of partial
translation units (PTUs). Every declaration will be associated with a unique PTU
that owns it.
The first key insight here is that the owning PTU isn't always the "active"
(most recent) PTU, and it isn't always the PTU that the declaration
"comes from". A new declaration (that isn't a redeclaration or specialization of
anything) does belong to the active PTU. A template specialization, however,
belongs to the most recent PTU of all the declarations in its signature - mostly
that means that it can be pulled into a more recent PTU by its template
arguments.
The second key insight is that processing a PTU might extend an earlier PTU.
Rolling back the later PTU shouldn't throw that extension away. For example, if
the second PTU defines a template, and the third PTU requires that template to
be instantiated at float, that template specialization is still part of the
second PTU. Similarly, if the fifth PTU uses an inline function belonging to the
fourth, that definition still belongs to the fourth. When we go to emit code in
a new PTU, we map each declaration we have to emit back to its owning PTU and
emit it in a new module for just the extensions to that PTU. We keep track of
all the modules we've emitted for a PTU so that we can unload them all if we
decide to roll it back.
Most declarations/definitions will only refer to entities from the same or
earlier PTUs. However, it is possible (primarily by defining a
previously-declared entity, but also through templates or ADL) for an entity
that belongs to one PTU to refer to something from a later PTU. We will have to
keep track of this and prevent unwinding to later PTU when we recognize it.
Fortunately, this should be very rare; and crucially, we don't have to do the
bookkeeping for this if we've only got one PTU, e.g. in normal compilation.
Otherwise, PTUs after the first just need to record enough metadata to be able
to revert any changes they've made to declarations belonging to earlier PTUs,
e.g. to redeclaration chains or template specialization lists.
It should even eventually be possible for PTUs to provide their own slab
allocators which can be thrown away as part of rolling back the PTU. We can
maintain a notion of the active allocator and allocate things like Stmt/Expr
nodes in it, temporarily changing it to the appropriate PTU whenever we go to do
something like instantiate a function template. More care will be required when
allocating declarations and types, though.
We would want the PTU to be efficiently recoverable from a Decl; I'm not sure
how best to do that. An easy option that would cover most declarations would be
to make multiple TranslationUnitDecls and parent the declarations appropriately,
but I don't think that's good enough for things like member function templates,
since an instantiation of that would still be parented by its original class.
Maybe we can work this into the DC chain somehow, like how lexical DCs are.
We add a different kind of translation unit `TU_Incremental` which is a
complete translation unit that we might nonetheless incrementally extend later.
Because it is complete (and we might want to generate code for it), we do
perform template instantiation, but because it might be extended later, we don't
warn if it declares or uses undefined internal-linkage symbols.
This patch teaches clang-repl how to recover from errors by disconnecting the
most recent PTU and update the primary PTU lookup tables. For instance:
```./clang-repl
clang-repl> int i = 12; error;
In file included from <<< inputs >>>:1:
input_line_0:1:13: error: C++ requires a type specifier for all declarations
int i = 12; error;
^
error: Parsing failed.
clang-repl> int i = 13; extern "C" int printf(const char*,...);
clang-repl> auto r1 = printf("i=%d\n", i);
i=13
clang-repl> quit
```
Differential revision: https://reviews.llvm.org/D104918
This reverts commit 6775fc6ffa.
It also reverts "[lldb] Fix compilation by adjusting to the new ASTContext signature."
This reverts commit 03a3f86071.
We see some failures on the lldb infrastructure, these changes might play a role
in it. Let's revert it now and see if the bots will become green.
Ref: https://reviews.llvm.org/D104918
https://reviews.llvm.org/D96033 contained a discussion regarding efficient
modeling of error recovery. @rjmccall has outlined the key ideas:
Conceptually, we can split the translation unit into a sequence of partial
translation units (PTUs). Every declaration will be associated with a unique PTU
that owns it.
The first key insight here is that the owning PTU isn't always the "active"
(most recent) PTU, and it isn't always the PTU that the declaration
"comes from". A new declaration (that isn't a redeclaration or specialization of
anything) does belong to the active PTU. A template specialization, however,
belongs to the most recent PTU of all the declarations in its signature - mostly
that means that it can be pulled into a more recent PTU by its template
arguments.
The second key insight is that processing a PTU might extend an earlier PTU.
Rolling back the later PTU shouldn't throw that extension away. For example, if
the second PTU defines a template, and the third PTU requires that template to
be instantiated at float, that template specialization is still part of the
second PTU. Similarly, if the fifth PTU uses an inline function belonging to the
fourth, that definition still belongs to the fourth. When we go to emit code in
a new PTU, we map each declaration we have to emit back to its owning PTU and
emit it in a new module for just the extensions to that PTU. We keep track of
all the modules we've emitted for a PTU so that we can unload them all if we
decide to roll it back.
Most declarations/definitions will only refer to entities from the same or
earlier PTUs. However, it is possible (primarily by defining a
previously-declared entity, but also through templates or ADL) for an entity
that belongs to one PTU to refer to something from a later PTU. We will have to
keep track of this and prevent unwinding to later PTU when we recognize it.
Fortunately, this should be very rare; and crucially, we don't have to do the
bookkeeping for this if we've only got one PTU, e.g. in normal compilation.
Otherwise, PTUs after the first just need to record enough metadata to be able
to revert any changes they've made to declarations belonging to earlier PTUs,
e.g. to redeclaration chains or template specialization lists.
It should even eventually be possible for PTUs to provide their own slab
allocators which can be thrown away as part of rolling back the PTU. We can
maintain a notion of the active allocator and allocate things like Stmt/Expr
nodes in it, temporarily changing it to the appropriate PTU whenever we go to do
something like instantiate a function template. More care will be required when
allocating declarations and types, though.
We would want the PTU to be efficiently recoverable from a Decl; I'm not sure
how best to do that. An easy option that would cover most declarations would be
to make multiple TranslationUnitDecls and parent the declarations appropriately,
but I don't think that's good enough for things like member function templates,
since an instantiation of that would still be parented by its original class.
Maybe we can work this into the DC chain somehow, like how lexical DCs are.
We add a different kind of translation unit `TU_Incremental` which is a
complete translation unit that we might nonetheless incrementally extend later.
Because it is complete (and we might want to generate code for it), we do
perform template instantiation, but because it might be extended later, we don't
warn if it declares or uses undefined internal-linkage symbols.
This patch teaches clang-repl how to recover from errors by disconnecting the
most recent PTU and update the primary PTU lookup tables. For instance:
```./clang-repl
clang-repl> int i = 12; error;
In file included from <<< inputs >>>:1:
input_line_0:1:13: error: C++ requires a type specifier for all declarations
int i = 12; error;
^
error: Parsing failed.
clang-repl> int i = 13; extern "C" int printf(const char*,...);
clang-repl> auto r1 = printf("i=%d\n", i);
i=13
clang-repl> quit
```
Differential revision: https://reviews.llvm.org/D104918
When creating a PCH file the use of a temp file will be dictated by the
presence or absence of the -fno-temp-file flag. Creating a module file
will always use a temp file via the new ForceUseTemporary flag.
This fixes bug 50033.
This patch https://reviews.llvm.org/D102876 caused some lit regressions on z/OS because tmp files were no longer being opened based on binary/text mode. This patch passes OpenFlags when creating tmp files so we can open files in different modes.
Reviewed By: amccarth
Differential Revision: https://reviews.llvm.org/D103806
incorrect std::string use. (Also remove redundant call to
RemoveFileOnSignal.)
Clang writes object files by first writing to a .tmp file and then
renaming to the final .obj name. On Windows, if a compile is killed
partway through the .tmp files don't get deleted.
Currently it seems like RemoveFileOnSignal takes care of deleting the
tmp files on Linux, but on Windows we need to call
setDeleteDisposition on tmp files so that they are deleted when
closed.
This patch switches to using TempFile to create the .tmp files we write
when creating object files, since it uses setDeleteDisposition on Windows.
This change applies to both Linux and Windows for consistency.
Differential Revision: https://reviews.llvm.org/D102876
This reverts commit 20797b129f.
Clang writes object files by first writing to a .tmp file and then
renaming to the final .obj name. On Windows, if a compile is killed
partway through the .tmp files don't get deleted.
Currently it seems like RemoveFileOnSignal takes care of deleting the
tmp files on Linux, but on Windows we need to call
setDeleteDisposition on tmp files so that they are deleted when
closed.
This patch switches to using TempFile to create the .tmp files we write
when creating object files, since it uses setDeleteDisposition on Windows.
This change applies to both Linux and Windows for consistency.
Differential Revision: https://reviews.llvm.org/D102876
There already exists cl_khr_fp64 extension. So OpenCL C 3.0
and higher should use the feature, earlier versions still
use the extension. OpenCL C 3.0 API spec states that extension
will be not described in the option string if corresponding
optional functionality is not supported (see 4.2. Querying Devices).
Due to that fact the usage of features for OpenCL C 3.0 must
be as follows:
```
$ clang -Xclang -cl-ext=+cl_khr_fp64,+__opencl_c_fp64 ...
$ clang -Xclang -cl-ext=-cl_khr_fp64,-__opencl_c_fp64 ...
```
e.g. the feature and the equivalent extension (if exists)
must be set to the same values
Reviewed By: Anastasia
Differential Revision: https://reviews.llvm.org/D96524
DisableGeneratingGlobalModuleIndex was being set by
CompilerInstance::findOrCompileModuleAndReadAST most of (but not all of)
the times it returned `nullptr` as a "normal" failure. Pull that up to
the caller, CompilerInstance::loadModule, to simplify the code. This
resolves a number of FIXMEs added during the refactoring in
5cca622310.
The extra cases where this is set are all some version of a fatal error,
and the only client of the field, shouldBuildGlobalModuleIndex, seems
to be unreachable in that case. Even if there is some corner case where
this has an effect, it seems like the right/consistent behaviour.
Differential Revision: https://reviews.llvm.org/D101672
Rename CompilerInstance's ModuleBuildFailed field to
DisableGeneratingGlobalModuleIndex, which more precisely describes its
role. Otherwise, it's hard to suss out how it's different from
ModuleLoader::HadFatalFailure, and what sort of code simplifications are
safe.
Differential Revision: https://reviews.llvm.org/D101670
5cca622310 refactored
CompilerInstance::loadModule, splitting out
findOrCompileModuleAndReadAST, but was careful to avoid making any
functional changes. It added ModuleLoader::OtherUncachedFailure to
facilitate this and left behind FIXMEs asking why certain failures
weren't cached.
After a closer look, I think we can just remove this and simplify the
code. This changes the behaviour of the following (simplified) code from
CompilerInstance::loadModule, causing a failure to be cached more often:
```
if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first))
return *MaybeModule;
if (ModuleName == getLangOpts().CurrentModule)
return MM.cacheModuleLoad(PP.lookupModule(...));
ModuleLoadResult Result = findOrCompileModuleAndReadAST(...);
if (Result.isNormal()) // This will be 'true' more often.
return MM.cacheModuleLoad(..., Module);
return Result;
```
`MM` here is a ModuleMap owned by the Preprocessor. Here are the cases
where `findOrCompileModuleAndReadAST` starts returning a "normal" failed
result:
- Emitted `diag::err_module_not_found`, where there's no module map
found.
- Emitted `diag::err_module_build_disabled`, where implicitly building
modules is disabled.
- Emitted `diag::err_module_cycle`, which detects module cycles in the
implicit modules build system.
- Emitted `diag::err_module_not_built`, which avoids building a module
in this CompilerInstance if another one tried and failed already.
- `compileModuleAndReadAST()` was called and failed to build.
The four errors are all fatal, and last item also reports a fatal error,
so it this extra caching has no functionality change... but even if it
did, it seems fine to cache these failed results within a ModuleMap
instance (note that each CompilerInstance has its own Preprocessor and
ModuleMap).
Differential Revision: https://reviews.llvm.org/D101667
Remove an early return from an `else` block that's immediately followed
by an equivalent early return after the `else` block.
Differential Revision: https://reviews.llvm.org/D101671
Language options are not available when a target is being created,
thus, a new method is introduced. Also, some refactoring is done,
such as removing OpenCL feature macros setting from TargetInfo.
Reviewed By: Anastasia
Differential Revision: https://reviews.llvm.org/D101087
D97493 separate target creation out to a single function
`CompilerInstance::createTarget`. However, it would overwrite AuxTarget
even if it has been set.
As @kadircet recommended in D98128, this patch check the existence of
AuxTarget and not overwrite it when it has been set.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D100024
Problem:
On SystemZ we need to open text files in text mode. On Windows, files opened in text mode adds a CRLF '\r\n' which may not be desirable.
Solution:
This patch adds two new flags
- OF_CRLF which indicates that CRLF translation is used.
- OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses CRLF translation.
Developers should now use either the OF_Text or OF_TextWithCRLF for text files and OF_None for binary files. If the developer doesn't want carriage returns on Windows, they should use OF_Text, if they do want carriage returns on Windows, they should use OF_TextWithCRLF.
So this is the behaviour per platform with my patch:
z/OS:
OF_None: open in binary mode
OF_Text : open in text mode
OF_TextWithCRLF: open in text mode
Windows:
OF_None: open file with no carriage return
OF_Text: open file with no carriage return
OF_TextWithCRLF: open file with carriage return
The Major change is in llvm/lib/Support/Windows/Path.inc to only set text mode if the OF_CRLF is set.
```
if (Flags & OF_CRLF)
CrtOpenFlags |= _O_TEXT;
```
These following files are the ones that still use OF_Text which I left unchanged. I modified all these except raw_ostream.cpp in recent patches so I know these were previously in Binary mode on Windows.
./llvm/lib/Support/raw_ostream.cpp
./llvm/lib/TableGen/Main.cpp
./llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
./llvm/unittests/Support/Path.cpp
./clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
./clang/lib/Frontend/CompilerInstance.cpp
./clang/lib/Driver/Driver.cpp
./clang/lib/Driver/ToolChains/Clang.cpp
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D99426
This patch consists of the initial changes to help distinguish between text and binary content correctly on z/OS. I would like to get feedback from Windows users on setting OF_None for all ToolOutputFiles. This seems to have been done as an optimization to prevent CRLF translation on Windows in the past.
Reviewed By: zibi
Differential Revision: https://reviews.llvm.org/D97785
As @sammccall mentioned in [[ https://reviews.llvm.org/D97109 | D97109 ]], I've extract the logic of creating Target and AuxTarget into a new function called `createTargetAndAuxTarget`.
Since there are many similar code in clang or other related tools, consolidating them into a single function may help others to maintain the logic handling target related things.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D97493
A module with errors would be marked as out-of-date, then the `compilerModule` action would produce it, but due to the error it would be treated as failure and the resulting PCM would not get used.
rdar://74087062
Differential Revision: https://reviews.llvm.org/D96246
Fix layering between `CompilerInstance::createDefaultOutputFile` and the
two versions of `createOutputFile`.
- Add missing configuration flags to `createDefaultOutputFile` so that
GeneratePCHAction and GenerateModuleFromModuleMapAction can use it.
They previously promised that temporary files were turned on; now
`createDefaultOutputFile` handles that logic.
- Lift the logic handling `InFile` and `Extension` to
`createDefaultOutputFile`, since it's only the callers of that
function that are using it.
- Rename the deeper of the two `createOutputFile`s to
`createOutputFileImpl` and make it private to `CompilerInstance` (to
prove that no one else is using it).
- Sink the logic for adding to `CompilerInstance::OutputFiles` down to
`createOutputFileImpl`, allowing two "optional" (but always used)
`std::string*` out parameters to be removed.
- Instead of passing a `std::error_code` out parameter into
`createOutputFileImpl`, have it return `Expected<>`.
- As a drive-by, inline `CompilerInstance::addOutputFile` into its only
caller, `createOutputFileImpl`.
Clean layering makes it easier for a future commit to extract
`createOutputFileImpl` out of `CompilerInstance`.
Differential Revision: https://reviews.llvm.org/D93248
Add a new `raw_pwrite_ostream` variant, `buffer_unique_ostream`, which
is like `buffer_ostream` but with unique ownership of the stream it's
wrapping. Use this in CompilerInstance to simplify the ownership of
non-seeking output streams, avoiding logic sprawled around to deal with
them specially.
This also simplifies future work to encapsulate output files in a
different class.
Differential Revision: https://reviews.llvm.org/D93260
Found this memory leak in `CompilerInstance::setVerboseOutputStream` by
inspection; it looks like this wasn't previously exercised, since it was
never called twice.
Differential Revision: https://reviews.llvm.org/D93249
This addresses an issue with how the PCH preable works, specifically:
1. When using a PCH/preamble the module hash changes and a different cache directory is used
2. When the preamble is used, PCH & PCM validation is disabled.
Due to combination of #1 and #2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date.
rdar://72611253
Differential Revision: https://reviews.llvm.org/D95159
Add support for stdin to SourceManager and FileManager. Adds
FileManager::getSTDIN, which adds a FileEntryRef for `<stdin>` and reads
the MemoryBuffer, which is stored as `FileEntry::Content`.
Eventually the other buffers in `ContentCache` will sink to here as well
-- we probably usually want to load/save a MemoryBuffer eagerly -- but
it's happening early for stdin to get rid of
CompilerInstance::InitializeSourceManager's final call to
`SourceManager::overrideFileContents`.
clang/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
relies on building a module from stdin; supporting that requires setting
ContentCache::BufferOverridden.
Differential Revision: https://reviews.llvm.org/D93148
Handle named pipes natively in SourceManager and FileManager, removing a
call to `SourceManager::overrideFileContents` in
`CompilerInstance::InitializeSourceManager` (removing a blocker for
sinking the content cache to FileManager (which will incidently sink
this new named pipe logic with it)).
SourceManager usually checks if the file entry's size matches the
eventually loaded buffer, but that's now skipped for named pipes since
the `stat` won't reflect the full size. Since we can't trust
`ContentsEntry->getSize()`, we also need shift the check for files that
are too large until after the buffer is loaded... and load the buffer
immediately in `createFileID` so that no client gets a bad value from
`ContentCache::getSize`. `FileManager::getBufferForFile` also needs to
treat these files as volatile when loading the buffer.
Native support in SourceManager / FileManager means that named pipes can
also be `#include`d, and clang/test/Misc/dev-fd-fs.c was expanded to
check for that.
This is a new version of 3b18a594c7, which
was reverted in b346322019 since it was
missing the `SourceManager` changes.
Differential Revision: https://reviews.llvm.org/D92531
Fix the comment in front of `compileModuleImpl`'s call to
`CompilerInstance::clearOutputFiles`. The purpose of this call is to
delete any stray temporary files after the module generation thread
crashes.
The comment is from f545f67de3, and
was associated with manually deleting a generated module map. Then
13afbf42d8 added this `clearOutputFiles`
call between the comment and the code it referenced. Finally,
1f76c4e810 started sending the generated
module map directly to the SourceManager instead of putting it on disk,
deleting the call that the comment referenced.
No functionality change.
Kazu Hirata