Skip to content

Commit ab658ce

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)
1 parent 02b64b7 commit ab658ce

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
@@ -154,6 +154,19 @@ struct PPCallbacksDependencyCollector : public DependencyCollector {
154154
PP.addPPCallbacks(std::move(CB));
155155
}
156156
};
157+
/// A utility for adding \c ASTReaderListener to a compiler instance at the
158+
/// appropriate time.
159+
struct ASTReaderListenerDependencyCollector : public DependencyCollector {
160+
using MakeL =
161+
llvm::unique_function<std::unique_ptr<ASTReaderListener>(ASTReader &R)>;
162+
MakeL Create;
163+
ASTReaderListenerDependencyCollector(MakeL Create) : Create(std::move(Create)) {}
164+
void attachToASTReader(ASTReader &R) final {
165+
std::unique_ptr<ASTReaderListener> L = Create(R);
166+
assert(L);
167+
R.addListener(std::move(L));
168+
}
169+
};
157170

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

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

325+
// Attach callback for modules loaded via PCH.
326+
if (!ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty()) {
327+
auto DC = std::make_shared<ASTReaderListenerDependencyCollector>(
328+
[&](ASTReader &R) {
329+
return std::make_unique<LookupPCHModulesListener>(R.getPreprocessor());
330+
});
331+
ScanInstance.addDependencyCollector(std::move(DC));
332+
}
333+
278334
// Enable caching in the resulting commands.
279335
ScanInstance.getFrontendOpts().CacheCompileJob = true;
280336
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)