This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
This patch changes the PCM serialization logic to refer to input files by their "requested" name. This fixes a bug where the dependency scanner reports the "final" file paths, which can result in failed explicit compiles due to the `module.modulemap` file not being surrounded by the expected framework directory structure.
Depends on D135634.
Reviewed By: benlangmuir, Bigcheese
Differential Revision: https://reviews.llvm.org/D135636
This reverts commit 67f34054d6.
This reapplies commit f99e5a9106.
This improves commit 8ab388e158 that unsucessfully attempted to forward-fix Windows test failure.
This is an attempt to fix a Windows bot failure. In the test introduced in 83973cf1, file dependencies were printed out-of-order (after replacing backslashes with slashes). This might've been caused by styles of some paths being different.
Before the fix the scanning would fail with
`-Werror,-Wnon-modular-include-in-module` despite the warning being
suppressed in the source code.
Existing approach with `-Wno-error` is not sufficient because it negates
only general `-Werror` but not specific `-Werror=...` and some warnings
can still emitted as errors. Make the approach stricter by using `-w`
flag and ignore all warnings, including those upgraded to errors. This
approach is still valid as it doesn't affect the dependencies.
rdar://101588531
Differential Revision: https://reviews.llvm.org/D138252
Based on include-cleaner's version, but:
- remove assert that can fail for input `/\<newline>* */`
- assert was also checking the wrong condition: that the prefix *differed* from
either `//` or from `/*`. Avoid use of strncmp where we can.
- add a comment that the brittleness of the text matching is intentional
Differential Revision: https://reviews.llvm.org/D138780
This patch replaces those occurrences of NoneType that would trigger
an error if the definition of NoneType were missing in None.h.
To keep this patch focused, I am deliberately not replacing None with
std::nullopt in this patch or updating comments. They will be
addressed in subsequent patches.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
Differential Revision: https://reviews.llvm.org/D138539
When a pcm has already been loaded from disk, reuse it from the
InMemoryModuleCache in readASTFileControlBlock. This avoids potentially
reading it again.
As noted in the FIXME, ideally we would also add the module to the cache
if it will be used again later, but that could modify its build state
and we do not have enough context currenlty to know if it's correct.
Differential Revision: https://reviews.llvm.org/D138160
Use a FileEntryRef when retrieving modulemap paths in the scanner so
that we use a path compatible with the original module import, rather
than a FileEntry which can allow unrelated modules to leak paths into
how we build a module due to FileManager mutating the path.
Note: the current change prevents an "unrelated" path, but does not
change how VFS mapped paths are handled (which would be calling
getNameAsRequested) nor canonicalize the path.
Differential Revision: https://reviews.llvm.org/D137989
Dotdots were removed only when converting a relative path to absolute
one. This patch makes the behaviour consistent for absolute file paths by
removing them in that case as well. Also updates the documentation to mention
the behaviour.
Fixes https://github.com/clangd/clangd/issues/1317
Differential Revision: https://reviews.llvm.org/D137962
We plan to reuse it in the include-cleaner library, this patch moves
this functionality from clangd to libtooling, so that this piece of code can be
shared among all clang tools.
Differential Revision: https://reviews.llvm.org/D137697
This was done as a test for D137302 and it makes sense to push these changes
Reviewed By: shafik
Differential Revision: https://reviews.llvm.org/D137491
Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.
This change enhances handling errors in the expansion so that users
could get more informative error messages.
Differential Revision: https://reviews.llvm.org/D136090
Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.
This change enhances handling errors in the expansion so that users
could get more informative error messages.
Differential Revision: https://reviews.llvm.org/D136090
This is a fix related to D135414. The original intention was to keep `BaseFS` as a member of the worker and conditionally overlay it with local in-memory FS. The `move` of ref-counted `BaseFS` was not intended, and it's a bug.
Disabling parallelism in the "by-module-name" test reliably reproduces this, and the test itself doesn't *need* parallelism. (I think `-j 4` was cargo culted from another test.) Reusing that test to check for correct behavior...
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D136124
The `-ivfsoverlay` flag was only being respected when the scanner was instructed to minimize the inputs. This patch respects that flag even in canonical preprocessing mode.
Depends on D135414.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D135416
The dependency scanning worker may create an in-memory VFS and inject that into its `FileManager`. (Implemented in D109485.) This VFS currently persists between scans, which is not correct: in-memory files created in one scan could affect subsequent scans. This patch changes things so that the in-memory VFS is local to `DependencyScanningWorker::computeDependencies()`.
To test this, one would first scan for dependencies of a named module and then run second scan (on the same worker) for something unrelated, which would pick up the in-memory file created in the first scan. This set up is impossible to achieve with `clang-scan-deps`. We could set this up in `ToolingTests`, but the scanner needs to access the physical filesystem to store `.pcm` files. AFAIK we don't have a good way to propagate something like `%t` into unit tests to make that feasible. (This could be achieved with something like the virtualized `OutputManager`.)
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D135414
Codegen options are typically unused by modules. Reset some of them to increase sharing between TUs with different flags.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D135720
As pointed out in https://reviews.llvm.org/D119130#3829816, this
introduces a clang AST dependency to the clangToolingInclusions, which is used
by clang-format.
Since rest of the inclusion tooling doesn't depend on clang ast, moving this
into a separate library.
Differential Revision: https://reviews.llvm.org/D135245
When dep-scanning, canonicalize the module map path as much as we can.
This avoids unnecessarily needing to build multiple versions of a module
due to symlinks or case-insensitive file paths.
Despite the name `tryGetRealPathName`, the previous implementation did
not actually return the realpath most of the time, and indeed it would
be incorrect to do so since the realpath could be outside the module
directory, which would have broken finding headers relative to the
module.
Instead, use a canonicalization that is specific to the needs of
modulemap files (canonicalize the directory separately from the
filename).
Differential Revision: https://reviews.llvm.org/D134923
Update SourceManager::ContentCache::OrigEntry to keep the original
FileEntryRef, and use that to enable ModuleMap::getModuleMapFile* to
return the original FileEntryRef. This change should be NFC for
most users of SourceManager::ContentCache, but it could affect behaviour
for users of getNameAsRequested such as in compileModuleImpl. I have not
found a way to detect that difference without additional functional
changes, other than incidental cases like changes from / to \ on
Windows so there is no new test.
Differential Revision: https://reviews.llvm.org/D135220
A few cases were not handled correctly. Notably:
#define ID(X) X
#define HIDE a ID(b)
HIDE
spelledForExpanded() would claim HIDE is an equivalent range of the 'b' it
contains, despite the fact that HIDE also covers 'a'.
While trying to fix this bug, I found findCommonRangeForMacroArgs hard
to understand (both the implementation and how it's used in spelledForExpanded).
It relies on details of the SourceLocation graph that are IMO fairly obscure.
So I've added/revised quite a lot of comments and made some naming tweaks.
Fixes https://github.com/clangd/clangd/issues/1289
Differential Revision: https://reviews.llvm.org/D134618
This patch provides `FileManager` with the CWD on construction in the worker, rather than later in the action.
Depends on D134976.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D134977
This patch removes the ability of a dependency scanning worker to share a `FileManager` instance between individual scans. It's not sound and doesn't provide performance benefits (due to the underlying caching VFS).
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D134976
This patch adds new member function to `DependencyScanningWorker` that allows clients to pass custom `DiagnosticConsumer`, and returns `bool`.
This provides more flexibility compared to the existing version that automatically stringifies diagnostics and returns them in `llvm::Error`.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D134838
Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.
This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.
No functional changes.
Differential Revision: https://reviews.llvm.org/D132379
Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.
This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.
No functional changes.
Differential Revision: https://reviews.llvm.org/D132379
In `ASTWriter`, input files are sorted based on whether they are system or user. The current implementation used single `std::queue` with `push_back` and `push_front`. This resulted in the user files being reversed.
This patch fixes that by keeping the system/user distinction, but otherwise serializing files in the order they were loaded by the `SourceManager`. This is then used in the dependency scanner to report module map dependencies in the correct order.
Depends on D134224.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D134248
Module map files describing excluded headers do affect compilation. Track them in the compiler, serialize them into the PCM file and report them in the scanner.
Depends on D134222.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D134224
This patch fixes compilation failure with explicit modules caused by scanner not reporting the module map describing the module whose implementation is being compiled.
Below is a breakdown of the attached test case. Note the VFS that makes frameworks "A" and "B" share the same header "shared/H.h".
In non-modular build, Clang skips the last import, since the "shared/H.h" header has already been included.
During scan (or implicit build), the compiler handles "tu.m" as follows:
* `@import B` imports module "B", as expected,
* `#import <A/H.h>` is resolved textually (due to `-fmodule-name=A`) to "shared/H.h" (due to the VFS remapping),
* `#import <B/H.h>` is resolved to import module "A_Private", since the header "shared/H.h" is already known to be part of that module, and the import is skipped.
In the end, the only modular dependency of the TU is "B".
In explicit modular build without `-fmodule-name=A`, TU does depend on module "A_Private" properly, not just textually. Clang therefore builds & loads its PCM, and knows to ignore the last import, since "shared/H.h" is known to be part of "A_Private".
But with current scanner behavior and `-fmodule-name=A` present, the last import fails during explicit build. Clang doesn't know about "A_Private" (it's included textually) and tries to import "B_Private" instead, which it doesn't know about either (the scanner correctly didn't report it as dependency). This is fixed by reporting the module map describing "A" and matching the semantics of implicit build.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D134222
The `ScanInstance` is a local variable in `DependencyScanningAction::runInvocation()` that is referenced by `ModuleDepCollector`. Since D132405, `ModuleDepCollector` can escape the function and can outlive its `ScanInstance`. This patch fixes that.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D133988
Attempt to fix the test failures observed in CI:
* Add Option dependency, which caused BUILD_SHARED_LIBS builds to fail
* Adapt tests that accidentally depended on the host platform: platforms
that don't use an integrated assembler (e.g. AIX) get a different set
of commands from the driver. Most dependency scanner tests can use
-fsyntax-only or -E instead of -c to avoid this, and in the rare case
we want to check -c specifically, set an explicit target so the
behaviour is independent of the host.
Original commit message follows.
---
Instead of trying to "fix" the original driver invocation by appending
arguments to it, split it into multiple commands, and for each -cc1
command use a CompilerInvocation to give precise control over the
invocation.
This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache), apply
optimizations, or start supporting multi-arch builds, which would
require different modules for each arch.
In the long run it may make sense to treat the TU commands as a
dependency graph, each with their own dependencies on modules or earlier
TU commands, but for now they are simply a list that is executed in
order, and the dependencies are simply duplicated. Since we currently
only support single-arch builds, there is no parallelism available in
the execution.
Differential Revision: https://reviews.llvm.org/D132405
Instead of trying to "fix" the original driver invocation by appending
arguments to it, split it into multiple commands, and for each -cc1
command use a CompilerInvocation to give precise control over the
invocation.
This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache), apply
optimizations, or start supporting multi-arch builds, which would
require different modules for each arch.
In the long run it may make sense to treat the TU commands as a
dependency graph, each with their own dependencies on modules or earlier
TU commands, but for now they are simply a list that is executed in
order, and the dependencies are simply duplicated. Since we currently
only support single-arch builds, there is no parallelism available in
the execution.
Differential Revision: https://reviews.llvm.org/D132405
Fangrui Song