[clang] 9dc0b16 - [clang][deps] Report module map describing compiled module
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 12:06:12 PDT 2022
Author: Jan Svoboda
Date: 2022-09-22T12:06:02-07:00
New Revision: 9dc0b1674841243110f95cdc9c9d97bcf30c1544
URL: https://github.com/llvm/llvm-project/commit/9dc0b1674841243110f95cdc9c9d97bcf30c1544
DIFF: https://github.com/llvm/llvm-project/commit/9dc0b1674841243110f95cdc9c9d97bcf30c1544.diff
LOG: [clang][deps] Report module map describing compiled module
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.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D134222
Added:
clang/test/ClangScanDeps/modules-header-sharing.m
Modified:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
compiler-rt/test/orc/TestCases/Darwin/x86-64/Inputs/dlopen-dlclose-x2.s
################################################################################
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 9facfb0beba26..aa0b6d4dc2748 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -505,6 +505,11 @@ class LangOptions : public LangOptionsBase {
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;
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index ae1359c356a25..22fcbf1269629 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2231,6 +2231,9 @@ class Preprocessor {
/// 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);
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 1aa2daa2c694a..1cc1dc310d46c 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -534,6 +534,13 @@ Module *Preprocessor::getCurrentModule() {
return getHeaderSearchInfo().lookupModule(getLangOpts().CurrentModule);
}
+Module *Preprocessor::getCurrentModuleImplementation() {
+ if (!getLangOpts().isCompilingModuleImplementation())
+ return nullptr;
+
+ return getHeaderSearchInfo().lookupModule(getLangOpts().ModuleName);
+}
+
//===----------------------------------------------------------------------===//
// Preprocessor Initialization Methods
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index bb6673d1373db..52062cdfb6502 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -189,6 +189,15 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
CI.clearImplicitModuleBuildOptions();
if (llvm::any_of(CI.getFrontendOpts().Inputs, needsModules)) {
+ Preprocessor &PP = ScanInstance.getPreprocessor();
+ if (Module *CurrentModule = PP.getCurrentModuleImplementation())
+ if (const FileEntry *CurrentModuleMap =
+ PP.getHeaderSearchInfo()
+ .getModuleMap()
+ .getModuleMapFileForUniquing(CurrentModule))
+ CI.getFrontendOpts().ModuleMapFiles.emplace_back(
+ CurrentModuleMap->getName());
+
SmallVector<ModuleID> DirectDeps;
for (const auto &KV : ModularDeps)
if (KV.second->ImportedByMainFile)
diff --git a/clang/test/ClangScanDeps/modules-header-sharing.m b/clang/test/ClangScanDeps/modules-header-sharing.m
new file mode 100644
index 0000000000000..0226e6b461962
--- /dev/null
+++ b/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
+ at 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
diff --git a/compiler-rt/test/orc/TestCases/Darwin/x86-64/Inputs/dlopen-dlclose-x2.s b/compiler-rt/test/orc/TestCases/Darwin/x86-64/Inputs/dlopen-dlclose-x2.s
deleted file mode 100644
index 3fbd1778878d6..0000000000000
--- a/compiler-rt/test/orc/TestCases/Darwin/x86-64/Inputs/dlopen-dlclose-x2.s
+++ /dev/null
@@ -1,90 +0,0 @@
-// Runs a sequence of dlopen, dlclose, dlopen, dlclose on a library "inits".
-// This is intended as a standard harness for testing constructor / destructor
-// behavior in the context of a full dlclose and then re-dlopen'ing of the
-// inits library.
-//
-// Compiled from:
-//
-// int main(int argc, char *argv[]) {
-// printf("entering main\n");
-// void *H = dlopen("inits", 0);
-// if (!H) {
-// printf("failed\n");
-// return -1;
-// }
-// if (dlclose(H) == -1) {
-// printf("failed\n");
-// return -1;
-// }
-// H = dlopen("inits", 0);
-// if (!H) {
-// printf("failed\n");
-// return -1;
-// }
-// if (dlclose(H) == -1) {
-// printf("failed\n");
-// return -1;
-// }
-// printf("leaving main\n");
-// return 0;
-//}
-
- .section __TEXT,__text,regular,pure_instructions
- .build_version macos, 13, 0 sdk_version 13, 0
- .globl _main
- .p2align 4, 0x90
-_main:
-
- pushq %r14
- pushq %rbx
- pushq %rax
- leaq L_str(%rip), %rdi
- callq _puts
- leaq L_.str.1(%rip), %rdi
- xorl %esi, %esi
- callq _dlopen
- movl $-1, %ebx
- leaq L_str.8(%rip), %r14
- testq %rax, %rax
- je LBB0_4
-
- movq %rax, %rdi
- callq _dlclose
- cmpl $-1, %eax
- je LBB0_4
-
- leaq L_.str.1(%rip), %rdi
- xorl %esi, %esi
- callq _dlopen
- testq %rax, %rax
- je LBB0_4
-
- movq %rax, %rdi
- callq _dlclose
- xorl %ebx, %ebx
- cmpl $-1, %eax
- sete %bl
- leaq L_str.8(%rip), %rax
- leaq L_str.6(%rip), %r14
- cmoveq %rax, %r14
- negl %ebx
-LBB0_4:
- movq %r14, %rdi
- callq _puts
- movl %ebx, %eax
- addq $8, %rsp
- popq %rbx
- popq %r14
- retq
-
- .section __TEXT,__cstring,cstring_literals
-L_.str.1:
- .asciz "inits"
-L_str:
- .asciz "entering main"
-L_str.6:
- .asciz "leaving main"
-L_str.8:
- .asciz "failed"
-
-.subsections_via_symbols
More information about the cfe-commits
mailing list