jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, Bigcheese.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134222
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-header-sharing.m
Index: clang/test/ClangScanDeps/modules-header-sharing.m
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-header-sharing.m
@@ -0,0 +1,96 @@
+// There are some edge-cases where Clang depends on knowing the module whose implementation it's currently building.
+// This test makes sure scanner always reports the corresponding module map.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- frameworks/A.framework/Modules/module.modulemap
+framework module A { umbrella header "A.h" }
+//--- frameworks/B.framework/Modules/module.modulemap
+framework module B { umbrella header "B.h" }
+//--- frameworks/A.framework/Headers/A.h
+//--- frameworks/B.framework/Headers/B.h
+//--- frameworks/A.framework/Modules/module.private.modulemap
+framework module A_Private { umbrella header "A_Private.h" }
+//--- frameworks/B.framework/Modules/module.private.modulemap
+framework module B_Private { umbrella header "B_Private.h" }
+//--- frameworks/A.framework/PrivateHeaders/A_Private.h
+#import <A/H.h>
+//--- frameworks/B.framework/PrivateHeaders/B_Private.h
+#import <B/H.h>
+
+//--- shared/H.h
+
+//--- overlay.json.template
+{
+ "case-sensitive": "false",
+ "version": 0,
+ "roots": [
+ {
+ "contents": [
+ {
+ "external-contents": "DIR/shared/H.h",
+ "name": "H.h",
+ "type": "file"
+ }
+ ],
+ "name": "DIR/frameworks/A.framework/PrivateHeaders",
+ "type": "directory"
+ },
+ {
+ "contents": [
+ {
+ "external-contents": "DIR/shared/H.h",
+ "name": "H.h",
+ "type": "file"
+ }
+ ],
+ "name": "DIR/frameworks/B.framework/PrivateHeaders",
+ "type": "directory"
+ }
+ ]
+}
+
+//--- cdb.json.template
+[{
+ "file": "DIR/tu.m",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=A -ivfsoverlay DIR/overlay.json -F DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
+}]
+
+//--- tu.m
+@import B;
+#import <A/H.h>
+#import <B/H.h>
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.template > %t/overlay.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+// CHECK: {
+// CHECK: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "commands": [
+// CHECK-NEXT: {
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/frameworks/A.framework/Modules/module.modulemap",
+// CHECK: "-fmodule-name=A",
+// CHECK: ],
+// CHECK-NEXT: "executable": "clang",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.m",
+// CHECK-NEXT: "[[PREFIX]]/shared/H.h"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+
+// RUN: %deps-to-rsp %t/result.json --module-name=B > %t/B.cc1.rsp
+// RUN: %clang @%t/B.cc1.rsp
+
+// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp
+// RUN: %clang @%t/tu.rsp
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -188,6 +188,14 @@
void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
CI.clearImplicitModuleBuildOptions();
+ Preprocessor &PP = ScanInstance.getPreprocessor();
+ if (Module *CurrentModule = PP.getCurrentModuleImplementation()) {
+ const FileEntry *CurrentModMap =
+ PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing(
+ CurrentModule);
+ CI.getFrontendOpts().ModuleMapFiles.emplace_back(CurrentModMap->getName());
+ }
+
if (llvm::any_of(CI.getFrontendOpts().Inputs, needsModules)) {
SmallVector<ModuleID> DirectDeps;
for (const auto &KV : ModularDeps)
Index: clang/lib/Lex/Preprocessor.cpp
===================================================================
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -534,6 +534,13 @@
return getHeaderSearchInfo().lookupModule(getLangOpts().CurrentModule);
}
+Module *Preprocessor::getCurrentModuleImplementation() {
+ if (!getLangOpts().isCompilingModuleImplementation())
+ return nullptr;
+
+ return getHeaderSearchInfo().lookupModule(getLangOpts().ModuleName);
+}
+
//===----------------------------------------------------------------------===//
// Preprocessor Initialization Methods
//===----------------------------------------------------------------------===//
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -2231,6 +2231,9 @@
/// Retrieves the module that we're currently building, if any.
Module *getCurrentModule();
+ /// Retrieves the module whose implementation we're current compiling, if any.
+ Module *getCurrentModuleImplementation();
+
/// Allocate a new MacroInfo object with the provided SourceLocation.
MacroInfo *AllocateMacroInfo(SourceLocation L);
Index: clang/include/clang/Basic/LangOptions.h
===================================================================
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -505,6 +505,11 @@
return getCompilingModule() == CMK_ModuleInterface;
}
+ /// Are we compiling a module implementation?
+ bool isCompilingModuleImplementation() const {
+ return !isCompilingModule() && !ModuleName.empty();
+ }
+
/// Do we need to track the owning module for a local declaration?
bool trackLocalOwningModule() const {
return isCompilingModule() || ModulesLocalVisibility;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
