unique_ptrify the ModuleManager's VisitState linked list

[clang] Use range-based for loops with llvm::reverse (NFC)

[clang] Use llvm::erase_if (NFC)

Change Module::ASTFile and ModuleFile::File => Optional<FileEntryRef>, NFC

Change `Module::ASTFile` and `ModuleFile::File` to use
`Optional<FileEntryRef>` instead of `const FileEntry *`. One of many

Change Module::ASTFile and ModuleFile::File => Optional<FileEntryRef>, NFC

Change `Module::ASTFile` and `ModuleFile::File` to use
`Optional<FileEntryRef>` instead of `const FileEntry *`. One of many
steps toward removing `FileEntry::getName`.

Differential Revision: https://reviews.llvm.org/D89836

show more ...

ModuleManager: Simplify lookupModuleFile by only setting the out parameter once, NFC

Differential Revision: https://reviews.llvm.org/D89835

Perform an extra consistency check when searching ModuleManager's
cache for implicit modules.

The ModuleManager's use of FileEntry nodes as the keys for its map of
loaded modules is less than ideal.

Perform an extra consistency check when searching ModuleManager's
cache for implicit modules.

The ModuleManager's use of FileEntry nodes as the keys for its map of
loaded modules is less than ideal. Uniqueness for FileEntry nodes is
maintained by FileManager, which in turn uses inode numbers on hosts
that support that. When coupled with the module cache's proclivity for
turning over and deleting stale PCMs, this means entries for different
module files can wind up reusing the same underlying inode. When this
happens, subsequent accesses to the Modules map will disagree on the
ModuleFile associated with a given file.

In general, it is not sufficient to resolve this conundrum with a type
like FileEntryRef that stores the name of the FileEntry node on first
access because of path canonicalization issues. However, the paths
constructed for implicit module builds are fully under Clang's
control. We *can*, therefore, rely on their structure being consistent
across operating systems and across subsequent accesses to the Modules
map.

To mitigate the effects of inode reuse, perform an extra name check when
implicit modules are returned from the cache. This has the effect of
forcing reused FileEntry nodes to stomp over existing-but-stale entries
in the cache, which simulates a miss - exactly the desired behavior.

rdar://48443680

Patch by Robert Widmann!

Differential Revision: https://reviews.llvm.org/D86823

show more ...

[clang] Use IsVolatile=true and RequiresNullTerminator=false for PCMs

This change got missed while upstreaming
https://reviews.llvm.org/D77772. This is the part of that change that
actually passes t

[clang] Use IsVolatile=true and RequiresNullTerminator=false for PCMs

This change got missed while upstreaming
https://reviews.llvm.org/D77772. This is the part of that change that
actually passes the correct arguments when opening a PCM.

The test didn't catch this because it starts at the
`MemoryBuffer::getOpenFile` level. It's not really possible to test
`ModuleManager::addModule` itself to verify how the file was opened.

show more ...

Revert "[modules] Do not cache invalid state for modules that we attempted to load."
As per comment on https://reviews.llvm.org/D72860, it is suggested to
revert this change in the meantime, since it

Revert "[modules] Do not cache invalid state for modules that we attempted to load."
As per comment on https://reviews.llvm.org/D72860, it is suggested to
revert this change in the meantime, since it has introduced regression.

This reverts commit 83f4c3af021cd5322ea10fd1c4e839874c1dae49.

show more ...

[NFC] Remove trailing space

sed -Ei 's/[[:space:]]+$//' include/**/*.{def,h,td} lib/**/*.{cpp,h,td}

[modules] Do not cache invalid state for modules that we attempted to load.

Partially reverts 0a2be46cfdb698fefcc860a56b47dde0884d5335 as it turned
out to cause redundant module rebuilds in multi-pr

[modules] Do not cache invalid state for modules that we attempted to load.

Partially reverts 0a2be46cfdb698fefcc860a56b47dde0884d5335 as it turned
out to cause redundant module rebuilds in multi-process incremental builds.
When a module was getting out of date, all compilation processes started at the
same time were marking it as `ToBuild`. So each process was building the same
module instead of checking if it was built by someone else and using that
result. In addition to the work duplication, contention on the same .pcm file
wasn't making builds faster.

Note that for a single-process build this change would cause redundant module
reads and validations. But reading a module is faster than building it and
multi-process builds are more common than single-process. So I'm willing to
make such a trade-off.

rdar://problem/54395127

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D72860

show more ...

clang/Modules: Move Serialization/Module.{h,cpp} to ModuleFile, NFC

Remove some cognitive load by renaming clang/Serialization/Module.h to
clang/Serialization/ModuleFile.h, since it declares the Mod

clang/Modules: Move Serialization/Module.{h,cpp} to ModuleFile, NFC

Remove some cognitive load by renaming clang/Serialization/Module.h to
clang/Serialization/ModuleFile.h, since it declares the ModuleFile
class. This also makes editing a bit easier, since the basename of the
file no long conflicts with clang/Basic/Module.h, which declares the
Module class. Also move lib/Serialization/Module.cpp to
lib/Serialization/ModuleFile.cpp.

show more ...

clang/Modules: Remove unused parameter from ModuleManager::removeModules

The other paremeters appear to be sufficient to determine which modules
have just been loaded and need to be removed, so stop

clang/Modules: Remove unused parameter from ModuleManager::removeModules

The other paremeters appear to be sufficient to determine which modules
have just been loaded and need to be removed, so stop collecting and
sending in that set explicitly.

show more ...

FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

Remove this dead code. We always close it.

llvm-svn: 370488

[Modules] Fix rebuilding an updated module for each of its consumers.

Marking a module for a rebuild when its signature differs from the
expected one causes redundant module rebuilds for incremental

[Modules] Fix rebuilding an updated module for each of its consumers.

Marking a module for a rebuild when its signature differs from the
expected one causes redundant module rebuilds for incremental builds.
When a module is updated, its signature changes. But its consumers still
have the old signature and loading them will result in signature
mismatches. It will correctly cause the rebuilds for the consumers but
we don't need to rebuild the common module for each of them as it is
already up to date.

In practice this bug causes longer build times. We are doing more work
than required and only a single process can build a module, so parallel
builds degrade to a single-process mode where extra processes are just
waiting on a file lock.

Fix by not marking a module dependency for a rebuild on signature
mismatch. We'll check if it is up to date when we load it.

rdar://problem/50212358

Reviewers: dexonsmith, bruno, rsmith

Reviewed By: dexonsmith, bruno

Subscribers: jkorous, ributzka, cfe-commits, aprantl

Differential Revision: https://reviews.llvm.org/D66907

llvm-svn: 370274

show more ...

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
o

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.

Differential revision: https://reviews.llvm.org/D66259

llvm-svn: 368942

show more ...

[clang] Adopt new FileManager error-returning APIs

Update the callers of FileManager::getFile and FileManager::getDirectory to handle the new llvm::ErrorOr-returning methods.

Signed-off-by: Harlan

[clang] Adopt new FileManager error-returning APIs

Update the callers of FileManager::getFile and FileManager::getDirectory to handle the new llvm::ErrorOr-returning methods.

Signed-off-by: Harlan Haskins <[email protected]>
llvm-svn: 367616

show more ...

Fix parameter name comments using clang-tidy. NFC.

This patch applies clang-tidy's bugprone-argument-comment tool
to LLVM, clang and lld source trees. Here is how I created this
patch:

$ git clone

Fix parameter name comments using clang-tidy. NFC.

This patch applies clang-tidy's bugprone-argument-comment tool
to LLVM, clang and lld source trees. Here is how I created this
patch:

$ git clone https://github.com/llvm/llvm-project.git
$ cd llvm-project
$ mkdir build
$ cd build
$ cmake -GNinja -DCMAKE_BUILD_TYPE=Debug \
-DLLVM_ENABLE_PROJECTS='clang;lld;clang-tools-extra' \
-DCMAKE_EXPORT_COMPILE_COMMANDS=On -DLLVM_ENABLE_LLD=On \
-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ ../llvm
$ ninja
$ parallel clang-tidy -checks='-*,bugprone-argument-comment' \
-config='{CheckOptions: [{key: StrictMode, value: 1}]}' -fix \
::: ../llvm/lib/**/*.{cpp,h} ../clang/lib/**/*.{cpp,h} ../lld/**/*.{cpp,h}

llvm-svn: 366177

show more ...

Range-style std::find{,_if} -> llvm::find{,_if}. NFC

llvm-svn: 357359

Modules: Invalidate out-of-date PCMs as they're discovered

Leverage the InMemoryModuleCache to invalidate a module the first time
it fails to import (and to lock a module as soon as it's built or
im

Modules: Invalidate out-of-date PCMs as they're discovered

Leverage the InMemoryModuleCache to invalidate a module the first time
it fails to import (and to lock a module as soon as it's built or
imported successfully). For implicit module builds, this optimizes
importing deep graphs where the leaf module is out-of-date; see example
near the end of the commit message.

Previously the cache finalized ("locked in") all modules imported so far
when starting a new module build. This was sufficient to prevent
loading two versions of the same module, but was somewhat arbitrary and
hard to reason about.

Now the cache explicitly tracks module state, where each module must be
one of:

- Unknown: module not in the cache (yet).
- Tentative: module in the cache, but not yet fully imported.
- ToBuild: module found on disk could not be imported; need to build.
- Final: module in the cache has been successfully built or imported.

Preventing repeated failed imports avoids variation in builds based on
shifting filesystem state. Now it's guaranteed that a module is loaded
from disk exactly once. It now seems safe to remove
FileManager::invalidateCache, but I'm leaving that for a later commit.

The new, precise logic uncovered a pre-existing problem in the cache:
the map key is the module filename, and different contexts use different
filenames for the same PCM file. (In particular, the test
Modules/relative-import-path.c does not build without this commit.
r223577 started using a relative path to describe a module's base
directory when importing it within another module. As a result, the
module cache sees an absolute path when (a) building the module or
importing it at the top-level, and a relative path when (b) importing
the module underneath another one.)

The "obvious" fix is to resolve paths using FileManager::getVirtualFile
and change the map key for the cache to a FileEntry, but some contexts
(particularly related to ASTUnit) have a shorter lifetime for their
FileManager than the InMemoryModuleCache. This is worth pursuing
further in a later commit; perhaps by tying together the FileManager and
InMemoryModuleCache lifetime, or moving the in-memory PCM storage into a
VFS layer.

For now, use the PCM's base directory as-written for constructing the
filename to check the ModuleCache.

Example
=======

To understand the build optimization, first consider the build of a
module graph TU -> A -> B -> C -> D with an empty cache:

TU builds A'
A' builds B'
B' builds C'
C' builds D'
imports D'
B' imports C'
imports D'
A' imports B'
imports C'
imports D'
TU imports A'
imports B'
imports C'
imports D'

If we build TU again, where A, B, C, and D are in the cache and D is
out-of-date, we would previously get this build:

TU imports A
imports B
imports C
imports D (out-of-date)
TU builds A'
A' imports B
imports C
imports D (out-of-date)
builds B'
B' imports C
imports D (out-of-date)
builds C'
C' imports D (out-of-date)
builds D'
imports D'
B' imports C'
imports D'
A' imports B'
imports C'
imports D'
TU imports A'
imports B'
imports C'
imports D'

After this commit, we'll immediateley invalidate A, B, C, and D when we
first observe that D is out-of-date, giving this build:

TU imports A
imports B
imports C
imports D (out-of-date)
TU builds A' // The same graph as an empty cache.
A' builds B'
B' builds C'
C' builds D'
imports D'
B' imports C'
imports D'
A' imports B'
imports C'
imports D'
TU imports A'
imports B'
imports C'
imports D'

The new build matches what we'd naively expect, pretty closely matching
the original build with the empty cache.

rdar://problem/48545366

llvm-svn: 355778

show more ...

Modules: Rename MemoryBufferCache to InMemoryModuleCache

Change MemoryBufferCache to InMemoryModuleCache, moving it from Basic to
Serialization. Another patch will start using it to manage module b

Modules: Rename MemoryBufferCache to InMemoryModuleCache

Change MemoryBufferCache to InMemoryModuleCache, moving it from Basic to
Serialization. Another patch will start using it to manage module build
more explicitly, but this is split out because it's mostly mechanical.

Because of the move to Serialization we can no longer abuse the
Preprocessor to forward it to the ASTReader. Besides the rename and
file move, that means Preprocessor::Preprocessor has one fewer parameter
and ASTReader::ASTReader has one more.

llvm-svn: 355777

show more ...

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the ne

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.

Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.

llvm-svn: 351636

show more ...

Move PCHContainerOperations from Frontend to Serialization

Fix a layering violation. Frontend depends on Serialization, so anything used
by both should be in Serialization.

llvm-svn: 348907

Lift VFS from clang to llvm (NFC)

This patch moves the virtual file system form clang to llvm so it can be
used by more projects.

Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang

Lift VFS from clang to llvm (NFC)

This patch moves the virtual file system form clang to llvm so it can be
used by more projects.

Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang/Basic to llvm/Support.
- Moves the corresponding unit test from clang to llvm.
- Moves the vfs namespace from clang::vfs to llvm::vfs.
- Formats the lines affected by this change, mostly this is the result of
the added llvm namespace.

RFC on the mailing list:
http://lists.llvm.org/pipermail/llvm-dev/2018-October/126657.html

Differential revision: https://reviews.llvm.org/D52783

llvm-svn: 344140

show more ...

Close FileEntries of cached files in ModuleManager::addModule().

While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
e

Close FileEntries of cached files in ModuleManager::addModule().

While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
errors, I found that most of them are .pcm files that are kept open by
ModuleManager. Pretty much all of the open file dscriptors are
FileEntries that are refering to `.pcm` files for which a buffer
already exists in a CompilerInstance's PCMCache.

Before PCMCache was added it was necessary to hold on to open file
descriptors to ensure that all ModuleManagers using the same
FileManager read the a consistent version of a given `.pcm` file on
disk, even when a concurrent clang process overwrites the file halfway
through. The PCMCache makes this practice unnecessary, since it caches
the entire contents of a `.pcm` file, while the FileManager caches all
the stat() information.

This patch adds a call to FileEntry::closeFile() to the path where a
Buffer has already been created. This is necessary because even for a
freshly written `.pcm` file the file is stat()ed once immediately
after writing to generate a FileEntry in the FileManager. Because a
freshly-generated file's contents is stored in the PCMCache, it is
fine to close the file immediately thereafter. The second change this
patch makes is to set the `ShouldClose` flag to true when reading a
`.pcm` file into the PCMCache for the first time.

[For reference, in 1 Clang instance there is
- 1 FileManager and
- n ModuleManagers with
- n PCMCaches.]

rdar://problem/40906753

Differential Revision: https://reviews.llvm.org/D50870

llvm-svn: 340188

show more ...

Remove trailing space

sed -Ei 's/[[:space:]]+$//' include/**/*.{def,h,td} lib/**/*.{cpp,h}

llvm-svn: 338291