Skip to content

Commit 47162cd

Browse files
committed
[clang][deps] Do check for relocated modules
This essentially reverts commit 227f719 (https://reviews.llvm.org/D150320). The original change to disable checking for relocated modules was built on the assumption that the file system is immutable and modules (their module maps) are not getting relocated. This turns out to not be true in Xcode projects where `.framework` directories may appear in a search path during the build. I was of the opinion then that this is a bug in the project build and that `.framework` directories appearing in a search path mid-build should not be observable (by either writing them at the very start of the build, or by not compiling any TUs that use the search path before populating it). This would result in the "module 'X' is defined in both 'MM1' and 'MM2'" error. While I still think this is a desirable property for a single-project build, multi-project workspace builds cannot avoid hitting this kind of issue and they are left with a build error they cannot fix. What's worse, since this is detected fairly late during deserialization (in `ReadASTBlock()`), Clang cannot graciously recover and rebuild the module. At this point, `ASTReader` is no longer able to remove pending module files (since they might've been partially deserialized). `Preprocessor` considers this situation a fatal error and tries to abort the compilation by calling `CurLexer->cutOffLexing()`. However, the dependency scanning lexer ignores this and continues to supply more tokens to the compiler. This may eventually lead to a crash caused by accessing the invalid `ASTReader` state. Fixing the dependency scanning lexer to respect the cut-off lead to more crashes somewhere during typo correction initiated by `Parser`/`Sema`. These issues should be fixed separately, but right now I'm mostly concerned about fixing multi-project workspace builds by being more lenient about relocated modules. With this patch, we're again catching the cases of one module being described by multiple different module map files early in `ReadASTCore()`. In that case, we mark the module chain as out of date, remove any pending module files, and try to rebuild the module. Note that this slows down scans by ~7.5%. I suspect we might be able to get some of that back by avoiding module map parsing in these relocation checks, and only looking at FS structure (at least for frameworks). rdar://133388373
1 parent 6ff33ed commit 47162cd

File tree

3 files changed

+37
-4
lines changed

3 files changed

+37
-4
lines changed

clang/include/clang/Driver/Options.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3353,7 +3353,7 @@ defm implicit_modules : BoolFOption<"implicit-modules",
33533353
NegFlag<SetFalse, [], [ClangOption, CC1Option]>,
33543354
PosFlag<SetTrue>, BothFlags<
33553355
[NoXarchOption], [ClangOption, CLOption]>>;
3356-
def fno_modules_check_relocated : Joined<["-"], "fno-modules-check-relocated">,
3356+
def fno_modules_check_relocated : Flag<["-"], "fno-modules-check-relocated">,
33573357
Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
33583358
HelpText<"Skip checks for relocated modules when loading PCM files">,
33593359
MarshallingInfoNegativeFlag<PreprocessorOpts<"ModulesCheckRelocated">>;

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,6 @@ class DependencyScanningAction : public tooling::ToolAction {
427427
ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings =
428428
true;
429429

430-
// Avoid some checks and module map parsing when loading PCM files.
431-
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;
432-
433430
std::unique_ptr<FrontendAction> Action;
434431

435432
if (Service.getFormat() == ScanningOutputFormat::P1689)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
// RUN: mkdir %t/frameworks1
5+
6+
// RUN: clang-scan-deps -format experimental-full -- \
7+
// RUN: %clang -fmodules -fmodules-cache-path=%t/cache \
8+
// RUN: -F %t/frameworks1 -F %t/frameworks2 \
9+
// RUN: -c %t/tu1.m -o %t/tu1.o
10+
11+
// RUN: cp -r %t/frameworks2/A.framework %t/frameworks1
12+
13+
// RUN: clang-scan-deps -format experimental-full -- \
14+
// RUN: %clang -fmodules -fmodules-cache-path=%t/cache \
15+
// RUN: -F %t/frameworks1 -F %t/frameworks2 \
16+
// RUN: -c %t/tu2.m -o %t/tu2.o
17+
18+
//--- frameworks2/A.framework/Modules/module.modulemap
19+
framework module A { header "A.h" }
20+
//--- frameworks2/A.framework/Headers/A.h
21+
#define MACRO_A 1
22+
23+
//--- frameworks2/B.framework/Modules/module.modulemap
24+
framework module B { header "B.h" }
25+
//--- frameworks2/B.framework/Headers/B.h
26+
#include <A/A.h>
27+
28+
//--- tu1.m
29+
#include <B/B.h>
30+
31+
//--- tu2.m
32+
#include <A/A.h>
33+
#include <B/B.h>
34+
35+
#if MACRO_A == 3
36+
#endif

0 commit comments

Comments
 (0)