Skip to content

Commit 50087ab

Browse files
committed
[clang][cas] Fix include-tree with pch importing public/private modules
We were dropping the modular import of a private module if its public module was imported via PCH, because we skipped parsing the private modulemap during scanning. (cherry picked from commit 6034ccd) (cherry picked from commit ab658ce)
1 parent 4b6a64e commit 50087ab

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed

clang/lib/Tooling/DependencyScanning/IncludeTreeActionController.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,19 @@ struct PPCallbacksDependencyCollector : public DependencyCollector {
153153
PP.addPPCallbacks(std::move(CB));
154154
}
155155
};
156+
/// A utility for adding \c ASTReaderListener to a compiler instance at the
157+
/// appropriate time.
158+
struct ASTReaderListenerDependencyCollector : public DependencyCollector {
159+
using MakeL =
160+
llvm::unique_function<std::unique_ptr<ASTReaderListener>(ASTReader &R)>;
161+
MakeL Create;
162+
ASTReaderListenerDependencyCollector(MakeL Create) : Create(std::move(Create)) {}
163+
void attachToASTReader(ASTReader &R) final {
164+
std::unique_ptr<ASTReaderListener> L = Create(R);
165+
assert(L);
166+
R.addListener(std::move(L));
167+
}
168+
};
156169

157170
struct IncludeTreePPCallbacks : public PPCallbacks {
158171
IncludeTreeBuilder &Builder;
@@ -217,6 +230,40 @@ struct IncludeTreePPCallbacks : public PPCallbacks {
217230
Builder.exitedSubmodule(PP, M, ImportLoc, ForPragma);
218231
}
219232
};
233+
234+
/// Utility to trigger module lookup in header search for modules loaded via
235+
/// PCH. This causes dependency scanning via PCH to parse modulemap files at
236+
/// roughly the same point they would with modulemap files embedded in the pcms,
237+
/// which is disabled with include-tree modules. Without this, we can fail to
238+
/// find modules that are in the same directory as a named import, since
239+
/// it may be skipped during search (see \c loadFrameworkModule).
240+
///
241+
/// The specific lookup we do matches what happens in ASTReader for the
242+
/// MODULE_DIRECTORY record, and ignores the result.
243+
class LookupPCHModulesListener : public ASTReaderListener {
244+
public:
245+
LookupPCHModulesListener(Preprocessor &PP) : PP(PP) {}
246+
247+
private:
248+
void ReadModuleName(StringRef ModuleName) final {
249+
if (PCHFinished)
250+
return;
251+
// Match MODULE_DIRECTORY: allow full search and ignore failure to find
252+
// the module.
253+
(void)PP.getHeaderSearchInfo().lookupModule(
254+
ModuleName, SourceLocation(), /*AllowSearch=*/true,
255+
/*AllowExtraModuleMapSearch=*/true);
256+
}
257+
void visitModuleFile(StringRef Filename,
258+
serialization::ModuleKind Kind) final {
259+
if (Kind == serialization::MK_PCH)
260+
PCHFinished = true;
261+
}
262+
263+
private:
264+
Preprocessor &PP;
265+
bool PCHFinished = false;
266+
};
220267
} // namespace
221268

222269
/// The PCH recorded file paths with canonical paths, create a VFS that
@@ -274,6 +321,15 @@ Error IncludeTreeActionController::initialize(
274321
});
275322
ScanInstance.addDependencyCollector(std::move(DC));
276323

324+
// Attach callback for modules loaded via PCH.
325+
if (!ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty()) {
326+
auto DC = std::make_shared<ASTReaderListenerDependencyCollector>(
327+
[&](ASTReader &R) {
328+
return std::make_unique<LookupPCHModulesListener>(R.getPreprocessor());
329+
});
330+
ScanInstance.addDependencyCollector(std::move(DC));
331+
}
332+
277333
// Enable caching in the resulting commands.
278334
ScanInstance.getFrontendOpts().CacheCompileJob = true;
279335
CASOpts = ScanInstance.getCASOpts();
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Test importing a private module whose public module was previously imported
2+
// via a PCH.
3+
4+
// REQUIRES: ondisk_cas
5+
6+
// RUN: rm -rf %t
7+
// RUN: split-file %s %t
8+
// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
9+
// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.template > %t/cdb_pch.json
10+
11+
// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json \
12+
// RUN: -cas-path %t/cas -module-files-dir %t/outputs \
13+
// RUN: -format experimental-include-tree-full -mode preprocess-dependency-directives \
14+
// RUN: > %t/deps_pch.json
15+
16+
// RUN: %deps-to-rsp %t/deps_pch.json --module-name Mod > %t/Mod.rsp
17+
// RUN: %deps-to-rsp %t/deps_pch.json --tu-index 0 > %t/pch.rsp
18+
// RUN: %clang @%t/Mod.rsp
19+
// RUN: %clang @%t/pch.rsp
20+
21+
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
22+
// RUN: -cas-path %t/cas -module-files-dir %t/outputs \
23+
// RUN: -format experimental-include-tree-full -mode preprocess-dependency-directives \
24+
// RUN: > %t/deps.json
25+
26+
// RUN: %deps-to-rsp %t/deps.json --module-name Mod_Private > %t/Mod_Private.rsp
27+
// RUN: %deps-to-rsp %t/deps.json --tu-index 0 > %t/tu.rsp
28+
// RUN: %clang @%t/Mod_Private.rsp
29+
// RUN: %clang @%t/tu.rsp
30+
31+
//--- cdb.json.template
32+
[{
33+
"file": "DIR/tu.m",
34+
"directory": "DIR",
35+
"command": "clang -fsyntax-only DIR/tu.m -include prefix.h -F DIR -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"
36+
}]
37+
38+
//--- cdb_pch.json.template
39+
[{
40+
"file": "DIR/prefix.h",
41+
"directory": "DIR",
42+
"command": "clang -x objective-c-header DIR/prefix.h -o DIR/prefix.h.pch -F DIR -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"
43+
}]
44+
45+
//--- Mod.framework/Modules/module.modulemap
46+
framework module Mod { header "Mod.h" }
47+
48+
//--- Mod.framework/Modules/module.private.modulemap
49+
framework module Mod_Private { header "Priv.h" }
50+
51+
//--- Mod.framework/Headers/Mod.h
52+
void pub(void);
53+
54+
//--- Mod.framework/PrivateHeaders/Priv.h
55+
void priv(void);
56+
57+
//--- prefix.h
58+
#import <Mod/Mod.h>
59+
60+
//--- tu.m
61+
#import <Mod/Priv.h>
62+
void tu(void) {
63+
pub();
64+
priv();
65+
}

0 commit comments

Comments
 (0)