Skip to content

Commit

Permalink
[clang][modules] Don't prevent translation of FW_Private includes whe…
Browse files Browse the repository at this point in the history 
…n explicitly building FW (#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
  • Loading branch information
@jansvoboda11
jansvoboda11 committed Nov 1, 2023
1 parent 243588d commit a3efd89
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 3 deletions.
7 changes: 4 additions & 3 deletions clang/lib/Basic/Module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
121 changes: 121 additions & 0 deletions clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
View file
Original file line number Diff line number Diff line change
@@ -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
@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

0 comments on commit a3efd89

Please sign in to comment.