Skip to content

Commit a39db6e

Browse files
jansvoboda11jansvoboda11
committed
[clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW (llvm#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 (cherry picked from commit a3efd89)
1 parent ac9bb6a commit a39db6e

File tree

2 files changed

+125
-3
lines changed

2 files changed

+125
-3
lines changed

‎clang/lib/Basic/Module.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,10 @@ bool Module::isForBuilding(const LangOptions &LangOpts) const {
166166
StringRef TopLevelName = getTopLevelModuleName();
167167
StringRef CurrentModule = LangOpts.CurrentModule;
168168

169-
// When building framework Foo, we want to make sure that Foo *and*
170-
// Foo_Private are textually included and no modules are built for both.
171-
if (getTopLevelModule()->IsFramework &&
169+
// When building the implementation of framework Foo, we want to make sure
170+
// that Foo *and* Foo_Private are textually included and no modules are built
171+
// for either.
172+
if (!LangOpts.isCompilingModule() && getTopLevelModule()->IsFramework &&
172173
CurrentModule == LangOpts.ModuleName &&
173174
!CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private"))
174175
TopLevelName = TopLevelName.drop_back(8);

‎clang/test/ClangScanDeps/modules-priv-fw-from-pub.m

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// This test checks that importing private headers from the public headers of
2+
// a framework is consistent between the dependency scanner and the explicit build.
3+
4+
// RUN: rm -rf %t
5+
// RUN: split-file %s %t
6+
7+
//--- frameworks/FW.framework/Modules/module.modulemap
8+
framework module FW { header "FW.h" }
9+
//--- frameworks/FW.framework/Modules/module.private.modulemap
10+
framework module FW_Private { header "FW_Private.h"}
11+
//--- frameworks/FW.framework/Headers/FW.h
12+
#include <FW/FW_Private.h>
13+
//--- frameworks/FW.framework/PrivateHeaders/FW_Private.h
14+
@import Dependency;
15+
16+
//--- modules/module.modulemap
17+
module Dependency { header "dependency.h" }
18+
//--- modules/dependency.h
19+
// empty
20+
21+
//--- tu.m
22+
#include <FW/FW.h>
23+
24+
//--- cdb.json.in
25+
[{
26+
"file": "DIR/tu.m",
27+
"directory": "DIR",
28+
"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"
29+
}]
30+
31+
// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
32+
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json
33+
34+
// Check that FW is reported to depend on FW_Private.
35+
// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
36+
// CHECK: {
37+
// CHECK-NEXT: "modules": [
38+
// CHECK-NEXT: {
39+
// CHECK-NEXT: "clang-module-deps": [],
40+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/modules/module.modulemap",
41+
// CHECK-NEXT: "command-line": [
42+
// CHECK: ],
43+
// CHECK-NEXT: "context-hash": "{{.*}}",
44+
// CHECK-NEXT: "file-deps": [
45+
// CHECK-NEXT: "[[PREFIX]]/modules/dependency.h",
46+
// CHECK-NEXT: "[[PREFIX]]/modules/module.modulemap"
47+
// CHECK-NEXT: ],
48+
// CHECK-NEXT: "name": "Dependency"
49+
// CHECK-NEXT: },
50+
// CHECK-NEXT: {
51+
// CHECK-NEXT: "clang-module-deps": [
52+
// CHECK-NEXT: {
53+
// CHECK-NEXT: "context-hash": "{{.*}}",
54+
// CHECK-NEXT: "module-name": "FW_Private"
55+
// CHECK-NEXT: }
56+
// CHECK-NEXT: ],
57+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
58+
// CHECK-NEXT: "command-line": [
59+
// CHECK: ],
60+
// CHECK-NEXT: "context-hash": "{{.*}}",
61+
// CHECK-NEXT: "file-deps": [
62+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
63+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
64+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
65+
// CHECK-NEXT: ],
66+
// CHECK-NEXT: "name": "FW"
67+
// CHECK-NEXT: },
68+
// CHECK-NEXT: {
69+
// CHECK-NEXT: "clang-module-deps": [
70+
// CHECK-NEXT: {
71+
// CHECK-NEXT: "context-hash": "{{.*}}",
72+
// CHECK-NEXT: "module-name": "Dependency"
73+
// CHECK-NEXT: }
74+
// CHECK-NEXT: ],
75+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
76+
// CHECK-NEXT: "command-line": [
77+
// CHECK: ],
78+
// CHECK-NEXT: "context-hash": "{{.*}}",
79+
// CHECK-NEXT: "file-deps": [
80+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
81+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
82+
// CHECK-NEXT: "[[PREFIX]]/modules/module.modulemap"
83+
// CHECK-NEXT: ],
84+
// CHECK-NEXT: "name": "FW_Private"
85+
// CHECK-NEXT: }
86+
// CHECK-NEXT: ],
87+
// CHECK-NEXT: "translation-units": [
88+
// CHECK-NEXT: {
89+
// CHECK-NEXT: "commands": [
90+
// CHECK-NEXT: {
91+
// CHECK-NEXT: "clang-context-hash": "{{.*}}",
92+
// CHECK-NEXT: "clang-module-deps": [
93+
// CHECK-NEXT: {
94+
// CHECK-NEXT: "context-hash": "{{.*}}",
95+
// CHECK-NEXT: "module-name": "FW"
96+
// CHECK-NEXT: }
97+
// CHECK-NEXT: ],
98+
// CHECK-NEXT: "command-line": [
99+
// CHECK: ],
100+
// CHECK-NEXT: "executable": "clang",
101+
// CHECK-NEXT: "file-deps": [
102+
// CHECK-NEXT: "[[PREFIX]]/tu.m"
103+
// CHECK-NEXT: ],
104+
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
105+
// CHECK-NEXT: }
106+
// CHECK-NEXT: ]
107+
// CHECK-NEXT: }
108+
// CHECK-NEXT: ]
109+
// CHECK-NEXT: }
110+
111+
// Check that building FW succeeds. If FW_Private was to be treated textually,
112+
// building FW would fail due to Dependency not being present on the command line.
113+
// RUN: %deps-to-rsp %t/deps.json --module-name=Dependency > %t/Dependency.cc1.rsp
114+
// RUN: %deps-to-rsp %t/deps.json --module-name=FW_Private > %t/FW_Private.cc1.rsp
115+
// RUN: %deps-to-rsp %t/deps.json --module-name=FW > %t/FW.cc1.rsp
116+
// RUN: %deps-to-rsp %t/deps.json --tu-index=0 > %t/tu.rsp
117+
118+
// RUN: %clang @%t/Dependency.cc1.rsp
119+
// RUN: %clang @%t/FW_Private.cc1.rsp
120+
// RUN: %clang @%t/FW.cc1.rsp
121+
// RUN: %clang @%t/tu.rsp

0 commit comments

Comments
 (0)