[clang] 94e64df - [clang][modules] Consider M affecting after mapping M.Private to M_Private
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 24 14:36:18 PDT 2022
Author: Jan Svoboda
Date: 2022-08-24T14:36:06-07:00
New Revision: 94e64df5763b49d750a9a87ecdd4a6583ad6154f
URL: https://github.com/llvm/llvm-project/commit/94e64df5763b49d750a9a87ecdd4a6583ad6154f
DIFF: https://github.com/llvm/llvm-project/commit/94e64df5763b49d750a9a87ecdd4a6583ad6154f.diff
LOG: [clang][modules] Consider M affecting after mapping M.Private to M_Private
When Clang encounters `@import M.Private` during implicit build, it precompiles module `M` and looks through its submodules. If the `Private` submodule is not found, Clang assumes `@import M_Private`. In the dependency scanner, we don't capture the dependency on `M`, since it's not imported. It's an affecting module, though: compilation of the import statement will fail when implicit modules are disabled and `M` is not precompiled and explicitly provided. This patch fixes that.
Depends on D132430.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D132502
Added:
clang/test/ClangScanDeps/modules-implicit-dot-private.m
Modified:
clang/include/clang/Lex/Preprocessor.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Lex/PPDirectives.cpp
clang/test/Modules/implicit-map-dot-private.m
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 5f54a44e604e2..470299b133a4a 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1335,6 +1335,16 @@ class Preprocessor {
/// \}
+ /// Mark the given module as affecting the current module or translation unit.
+ void markModuleAsAffecting(Module *M) {
+ if (!BuildingSubmoduleStack.empty()) {
+ if (M != BuildingSubmoduleStack.back().M)
+ BuildingSubmoduleStack.back().M->AffectingModules.insert(M);
+ } else {
+ AffectingModules.insert(M);
+ }
+ }
+
/// Get the set of top-level modules that affected preprocessing, but were not
/// imported.
const llvm::SmallSetVector<Module *, 2> &getAffectingModules() const {
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 6d1e378bf4dc1..c44ee00d90ffc 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -2013,8 +2013,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
// match Foo_Private and emit a warning asking for the user to write
// @import Foo_Private instead. FIXME: remove this when existing clients
// migrate off of Foo.Private syntax.
- if (!Sub && PP->getLangOpts().ImplicitModules && Name == "Private" &&
- Module == Module->getTopLevelModule()) {
+ if (!Sub && Name == "Private" && Module == Module->getTopLevelModule()) {
SmallString<128> PrivateModule(Module->Name);
PrivateModule.append("_Private");
@@ -2028,6 +2027,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
Sub = loadModule(ImportLoc, PrivPath, Visibility, IsInclusionDirective);
if (Sub) {
MapPrivateSubModToTopLevel = true;
+ PP->markModuleAsAffecting(Module);
if (!getDiagnostics().isIgnored(
diag::warn_no_priv_submodule_use_toplevel, ImportLoc)) {
getDiagnostics().Report(Path[I].second,
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 716f866e139ab..f5a9698c57c07 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2281,13 +2281,8 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
if (Imported) {
Action = Import;
} else if (Imported.isMissingExpected()) {
- Module *M = static_cast<Module *>(Imported)->getTopLevelModule();
- if (!BuildingSubmoduleStack.empty()) {
- if (Imported != BuildingSubmoduleStack.back().M)
- BuildingSubmoduleStack.back().M->AffectingModules.insert(M);
- } else {
- AffectingModules.insert(M);
- }
+ markModuleAsAffecting(
+ static_cast<Module *>(Imported)->getTopLevelModule());
// We failed to find a submodule that we assumed would exist (because it
// was in the directory of an umbrella header, for instance), but no
// actual module containing it exists (because the umbrella header is
diff --git a/clang/test/ClangScanDeps/modules-implicit-dot-private.m b/clang/test/ClangScanDeps/modules-implicit-dot-private.m
new file mode 100644
index 0000000000000..6ae8eb2947067
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-implicit-dot-private.m
@@ -0,0 +1,85 @@
+// This test checks that modules loaded during compilation (but not imported)
+// are still reported as dependencies.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- frameworks/FW.framework/Modules/module.modulemap
+framework module FW { umbrella header "FW.h" }
+//--- frameworks/FW.framework/Headers/FW.h
+//--- frameworks/FW.framework/Modules/module.private.modulemap
+framework module FW_Private { umbrella header "FW_Private.h" }
+//--- frameworks/FW.framework/PrivateHeaders/FW_Private.h
+
+//--- cdb.json.template
+[{
+ "file": "DIR/tu.m",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
+}]
+//--- tu.m
+ at import FW.Private;
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.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-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "FW"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "FW_Private"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-context-hash": "{{.*}}",
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "FW"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "FW_Private"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "command-line": [
+// CHECK: "-fmodule-file={{.*}}/FW_Private-{{.*}}.pcm"
+// CHECK: "-fmodule-file={{.*}}/FW-{{.*}}.pcm"
+// CHECK: ],
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.m"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+
+// RUN: %deps-to-rsp %t/result.json --module-name=FW > %t/FW.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=FW_Private > %t/FW_Private.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp
+// RUN: %clang @%t/FW.cc1.rsp
+// RUN: %clang @%t/FW_Private.cc1.rsp
+// RUN: %clang @%t/tu.rsp
diff --git a/clang/test/Modules/implicit-map-dot-private.m b/clang/test/Modules/implicit-map-dot-private.m
index 9fd8677f9e700..6770a8cad79e0 100644
--- a/clang/test/Modules/implicit-map-dot-private.m
+++ b/clang/test/Modules/implicit-map-dot-private.m
@@ -1,5 +1,17 @@
// RUN: rm -rf %t
-// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-canonical -emit-pch -o %t-A.pch %s -Wprivate-module
+
+// Implicit modules.
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -F %S/Inputs/implicit-private-canonical -fsyntax-only %s -Wprivate-module
+
+// Explicit modules.
+// RUN: %clang_cc1 -x objective-c -fmodules -emit-module -fmodule-name=A -o %t/A.pcm \
+// RUN: %S/Inputs/implicit-private-canonical/A.framework/Modules/module.modulemap -Wprivate-module
+// RUN: %clang_cc1 -x objective-c -fmodules -emit-module -fmodule-name=A_Private -o %t/A_Private.pcm \
+// RUN: %S/Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap -Wprivate-module
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fno-implicit-modules \
+// RUN: -fmodule-file=A=%t/A.pcm -fmodule-file=A_Private=%t/A_Private.pcm \
+// RUN: -F %S/Inputs/implicit-private-canonical -fsyntax-only %s -Wprivate-module
#ifndef HEADER
#define HEADER
More information about the cfe-commits
mailing list