[clang] [clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW (PR #70714)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 30 13:06:41 PDT 2023
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/70714
We prevent translating `#include <FW/PrivateHeader.h>` into an import of FW_Private when compiling the implementation of FW or FW_Private. This is specified via `-fmodule-name=` on the TU command line (used to be `-fmodule-implementation-of`).
This logic is supposed to only kick in when imported directly from a TU, but it currently also kicks in when compiling the public FW module explicitly (since it also has `-fmodule-name=` on the command line).
This patch makes sure this logic only kicks in for the case that used to be `-fmodule-implementation-of` (for the TU), and not for all `-fmodule-name=` cases (especially for the explicit compile of a module).
rdar://101051277; related: rdar://37500098&38434694
>From 8c2bbe27909fa6dbf0aac2c2b856a30e72046b2f Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 30 Oct 2023 11:44:56 -0700
Subject: [PATCH] [clang][modules] Don't prevent translation of FW_Private
includes when explicitly building FW
We prevent translating `#include <FW/PrivateHeader.h>` into an import of FW_Private when compiling the implementation of FW or FW_Private. This is specified via `-fmodule-name=` on the TU command line (used to be `-fmodule-implementation-of`).
This logic is supposed to only kick in when imported directly from a TU, but it currently also kicks in when compiling the public FW module explicitly (since it also has `-fmodule-name=` on the command line).
This patch makes sure this logic only kicks in for the case that used to be `-fmodule-implementation-of` (for the TU), and not for all `-fmodule-name=` cases (especially for the explicit compile of a module).
rdar://101051277; related: rdar://37500098&38434694
---
clang/lib/Basic/Module.cpp | 7 +-
.../ClangScanDeps/modules-priv-fw-from-pub.m | 121 ++++++++++++++++++
2 files changed, 125 insertions(+), 3 deletions(-)
create mode 100644 clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index cc2e5be98d32d34..e4ac1abf12a7f8e 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -161,9 +161,10 @@ bool Module::isForBuilding(const LangOptions &LangOpts) const {
StringRef TopLevelName = getTopLevelModuleName();
StringRef CurrentModule = LangOpts.CurrentModule;
- // When building framework Foo, we want to make sure that Foo *and*
- // Foo_Private are textually included and no modules are built for both.
- if (getTopLevelModule()->IsFramework &&
+ // When building the implementation of framework Foo, we want to make sure
+ // that Foo *and* Foo_Private are textually included and no modules are built
+ // for either.
+ if (!LangOpts.isCompilingModule() && getTopLevelModule()->IsFramework &&
CurrentModule == LangOpts.ModuleName &&
!CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private"))
TopLevelName = TopLevelName.drop_back(8);
diff --git a/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
new file mode 100644
index 000000000000000..cb82e0c366322c9
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
@@ -0,0 +1,121 @@
+// This test checks that importing private headers from the public headers of
+// a framework is consistent between the dependency scanner and the explicit build.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- frameworks/FW.framework/Modules/module.modulemap
+framework module FW { header "FW.h" }
+//--- frameworks/FW.framework/Modules/module.private.modulemap
+framework module FW_Private { header "FW_Private.h"}
+//--- frameworks/FW.framework/Headers/FW.h
+#include <FW/FW_Private.h>
+//--- frameworks/FW.framework/PrivateHeaders/FW_Private.h
+ at import Dependency;
+
+//--- modules/module.modulemap
+module Dependency { header "dependency.h" }
+//--- modules/dependency.h
+// empty
+
+//--- tu.m
+#include <FW/FW.h>
+
+//--- cdb.json.in
+[{
+ "file": "DIR/tu.m",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -I DIR/modules -F DIR/frameworks -Wno-framework-include-private-from-public -Wno-atimport-in-framework-header -c DIR/tu.m -o DIR/tu.o"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json
+
+// Check that FW is reported to depend on FW_Private.
+// RUN: cat %t/deps.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]]/modules/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/modules/dependency.h",
+// CHECK-NEXT: "[[PREFIX]]/modules/module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "Dependency"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "FW_Private"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// 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: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "Dependency"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// 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.private.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
+// CHECK-NEXT: "[[PREFIX]]/modules/module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "FW_Private"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "commands": [
+// 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: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "executable": "clang",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.m"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+
+// Check that building FW succeeds. If FW_Private was to be treated textually,
+// building FW would fail due to Dependency not being present on the command line.
+// RUN: %deps-to-rsp %t/deps.json --module-name=Dependency > %t/Dependency.cc1.rsp
+// RUN: %deps-to-rsp %t/deps.json --module-name=FW_Private > %t/FW_Private.cc1.rsp
+// RUN: %deps-to-rsp %t/deps.json --module-name=FW > %t/FW.cc1.rsp
+// RUN: %deps-to-rsp %t/deps.json --tu-index=0 > %t/tu.rsp
+
+// RUN: %clang @%t/Dependency.cc1.rsp
+// RUN: %clang @%t/FW_Private.cc1.rsp
+// RUN: %clang @%t/FW.cc1.rsp
+// RUN: %clang @%t/tu.rsp
More information about the cfe-commits
mailing list