Skip to content

Commit d844c16

Browse files
committed
[clang][cas] Correctly split out modules that are only imported for visibility
Modules imported for visibility only can happen when -fmodule-name matches a modular header that is imported more than once so that we trigger the correct re-exports but without trying to import a module.
1 parent 9565950 commit d844c16

File tree

10 files changed

+63
-20
lines changed

10 files changed

+63
-20
lines changed

clang/include/clang/Basic/DiagnosticLexKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,9 @@ def warn_defined_in_function_type_macro : Extension<
932932
"macro expansion producing 'defined' has undefined behavior">,
933933
InGroup<ExpansionToDefined>;
934934

935+
def err_pp_missing_module_include_tree : Error<
936+
"no module named '%0' declared in include-tree module map">, DefaultFatal;
937+
935938
let CategoryName = "Nullability Issue" in {
936939

937940
def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;

clang/include/clang/CAS/IncludeTree.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,17 +331,20 @@ class IncludeTree::ModuleImport : public IncludeTreeBase<ModuleImport> {
331331
public:
332332
static constexpr StringRef getNodeKind() { return "ModI"; }
333333

334-
static Expected<ModuleImport> create(ObjectStore &DB, StringRef ModuleName);
334+
static Expected<ModuleImport> create(ObjectStore &DB, StringRef ModuleName,
335+
bool VisibilityOnly);
335336

336-
StringRef getModuleName() { return getData(); }
337+
StringRef getModuleName() const { return getData().drop_front(); }
338+
/// Whether this module should only be "marked visible" rather than imported.
339+
bool visibilityOnly() const { return (bool)getData()[0]; }
337340

338341
llvm::Error print(llvm::raw_ostream &OS, unsigned Indent = 0);
339342

340343
static bool isValid(const ObjectProxy &Node) {
341344
if (!IncludeTreeBase::isValid(Node))
342345
return false;
343346
IncludeTreeBase Base(Node);
344-
return Base.getNumReferences() == 0 && !Base.getData().empty();
347+
return Base.getNumReferences() == 0 && Base.getData().size() > 1;
345348
}
346349
static bool isValid(ObjectStore &DB, ObjectRef Ref) {
347350
auto Node = DB.getProxy(Ref);

clang/include/clang/Lex/PPCachedActions.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ class PPCachedActions {
3939
/// The module that is imported by an \c #include directive or \c @import.
4040
struct IncludeModule {
4141
SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> ImportPath;
42+
// Whether this module should only be "marked visible" rather than imported.
43+
bool VisibilityOnly;
4244
};
4345

4446
virtual ~PPCachedActions() = default;

clang/lib/CAS/IncludeTree.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,12 @@ bool IncludeTree::isValid(const ObjectProxy &Node) {
212212
}
213213

214214
Expected<IncludeTree::ModuleImport>
215-
IncludeTree::ModuleImport::create(ObjectStore &DB, StringRef ModuleName) {
216-
return IncludeTreeBase::create(DB, {},
217-
llvm::arrayRefFromStringRef<char>(ModuleName));
215+
IncludeTree::ModuleImport::create(ObjectStore &DB, StringRef ModuleName,
216+
bool VisibilityOnly) {
217+
SmallString<64> Buffer;
218+
Buffer.push_back((char)VisibilityOnly);
219+
Buffer.append(ModuleName);
220+
return IncludeTreeBase::create(DB, {}, Buffer);
218221
}
219222

220223
IncludeTree::FileList::FileSizeTy
@@ -600,7 +603,11 @@ llvm::Error IncludeTree::FileList::print(llvm::raw_ostream &OS,
600603

601604
llvm::Error IncludeTree::ModuleImport::print(llvm::raw_ostream &OS,
602605
unsigned Indent) {
603-
OS << "(Module) " << getModuleName() << '\n';
606+
if (visibilityOnly())
607+
OS << "(Module for visibility only) ";
608+
else
609+
OS << "(Module) ";
610+
OS << getModuleName() << '\n';
604611
return llvm::Error::success();
605612
}
606613

clang/lib/Frontend/IncludeTreePPActions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ class IncludeTreePPActions final : public PPCachedActions {
123123
Import.getModuleName().split(ModuleComponents, '.');
124124
for (StringRef Component : ModuleComponents)
125125
Path.emplace_back(PP.getIdentifierInfo(Component), IncludeLoc);
126-
return IncludeModule{std::move(Path)};
126+
return IncludeModule{std::move(Path), Import.visibilityOnly()};
127127
}
128128

129129
assert(Node->getKind() == cas::IncludeTree::NodeKind::Tree);

clang/lib/Lex/PPDirectives.cpp

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,17 +2093,36 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
20932093
return;
20942094
}
20952095
if (auto *Import = std::get_if<PPCachedActions::IncludeModule>(&Include)) {
2096-
ModuleLoadResult Imported = TheModuleLoader.loadModule(
2097-
IncludeTok.getLocation(), Import->ImportPath, Module::Hidden,
2098-
/*IsIncludeDirective=*/true);
2099-
if (!Imported) {
2100-
assert(hadModuleLoaderFatalFailure() && "unexpected failure kind");
2101-
if (hadModuleLoaderFatalFailure()) {
2102-
IncludeTok.setKind(tok::eof);
2103-
CurLexer->cutOffLexing();
2096+
ModuleLoadResult Imported;
2097+
if (Import->VisibilityOnly) {
2098+
ModuleMap &MMap = getHeaderSearchInfo().getModuleMap();
2099+
Module *M = nullptr;
2100+
for (auto &NameLoc : Import->ImportPath) {
2101+
M = MMap.lookupModuleQualified(NameLoc.first->getName(), M);
2102+
if (!M)
2103+
break;
2104+
}
2105+
if (!M) {
2106+
Diags->Report(diag::err_pp_missing_module_include_tree)
2107+
<< getLangOpts().CurrentModule;
2108+
2109+
return;
2110+
}
2111+
Imported = M;
2112+
} else {
2113+
Imported = TheModuleLoader.loadModule(
2114+
IncludeTok.getLocation(), Import->ImportPath, Module::Hidden,
2115+
/*IsIncludeDirective=*/true);
2116+
if (!Imported) {
2117+
assert(hadModuleLoaderFatalFailure() && "unexpected failure kind");
2118+
if (hadModuleLoaderFatalFailure()) {
2119+
IncludeTok.setKind(tok::eof);
2120+
CurLexer->cutOffLexing();
2121+
}
2122+
return;
21042123
}
2105-
return;
21062124
}
2125+
21072126
makeModuleVisible(Imported, EndLoc);
21082127
if (IncludeTok.getIdentifierInfo()->getPPKeywordID() !=
21092128
tok::pp___include_macros)

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,10 @@ class DependencyScanningAction : public tooling::ToolAction {
466466
else
467467
Action = std::make_unique<ReadPCHAndPreprocessAction>();
468468

469+
// Normally this would be handled by GeneratePCHAction
470+
if (ScanInstance.getFrontendOpts().ProgramAction == frontend::GeneratePCH)
471+
ScanInstance.getLangOpts().CompilingPCH = true;
472+
469473
if (Error E = Controller.initialize(ScanInstance, OriginalInvocation))
470474
return reportError(std::move(E));
471475

clang/lib/Tooling/DependencyScanning/IncludeTreeActionController.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,9 @@ void IncludeTreeBuilder::handleHasIncludeCheck(Preprocessor &PP, bool Result) {
398398

399399
void IncludeTreeBuilder::moduleImport(Preprocessor &PP, const Module *M,
400400
SourceLocation EndLoc) {
401-
auto Import =
402-
check(cas::IncludeTree::ModuleImport::create(DB, M->getFullModuleName()));
401+
bool VisibilityOnly = M->isForBuilding(PP.getLangOpts());
402+
auto Import = check(cas::IncludeTree::ModuleImport::create(
403+
DB, M->getFullModuleName(), VisibilityOnly));
403404
if (!Import)
404405
return;
405406

clang/test/ClangScanDeps/modules-include-tree-implementation-private.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
// CHECK: Submodule: Mod
2424
// CHECK: 3:1 [[PREFIX]]/Mod.framework/PrivateHeaders/Priv.h llvmcas://
2525
// CHECK: Submodule: Mod_Private
26+
// CHECK: 4:1 (Module for visibility only) Mod
27+
// CHECK: 5:1 (Module for visibility only) Mod_Private
2628
// CHECK: Module Map:
2729
// CHECK: Mod (framework)
2830
// CHECK: link Mod (framework)
@@ -59,6 +61,8 @@ void priv(void);
5961
//--- tu.m
6062
#import <Mod/Mod.h>
6163
#import <Mod/Priv.h>
64+
#import <Mod/Mod.h>
65+
#import <Mod/Priv.h>
6266
void tu(void) {
6367
pub();
6468
priv();

clang/test/ClangScanDeps/modules-include-tree-implementation.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
// CHECK: 2:1 [[PREFIX]]/Mod.h llvmcas://{{[[:xdigit:]]+}}
2929
// CHECK: Submodule: Mod
30-
// CHECK: 3:1 (Module) Mod
30+
// CHECK: 3:1 (Module for visibility only) Mod
3131

3232
// CHECK: Files:
3333
// CHECK: [[PREFIX]]/tu.c llvmcas://{{[[:xdigit:]]+}}

0 commit comments

Comments
 (0)