Skip to content

Commit f33173a

Browse files
jansvoboda11jansvoboda11
committed
[clang][modules][deps] System module maps might not be affecting
The dependency scanner relies on the module map filtering logic in `ASTWriter`. The algorithm currently considers all system module maps affecting, which is not only sub-optimal, but can also cause failures when building a module explicitly (see attached test case). This patch applies the same filtering logic to system module maps. Reviewed By: Bigcheese Differential Revision: https://reviews.llvm.org/D136007
1 parent b4e1466 commit f33173a

File tree

4 files changed

+67
-13
lines changed

4 files changed

+67
-13
lines changed

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4544,7 +4544,7 @@ void ASTWriter::collectNonAffectingInputFiles() {
45444544
continue;
45454545

45464546
if (!isModuleMap(File.getFileCharacteristic()) ||
4547-
isSystem(File.getFileCharacteristic()) || AffectingModuleMaps.empty() ||
4547+
AffectingModuleMaps.empty() ||
45484548
AffectingModuleMaps.find(Cache->OrigEntry) != AffectingModuleMaps.end())
45494549
continue;
45504550

‎clang/test/ClangScanDeps/modules-implicit-dot-private.m

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
[{
1616
"file": "DIR/tu.m",
1717
"directory": "DIR",
18-
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
18+
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
1919
}]
2020
//--- tu.m
2121
@import FW.Private;
@@ -33,8 +33,7 @@
3333
// CHECK-NEXT: "context-hash": "{{.*}}",
3434
// CHECK: "file-deps": [
3535
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
36-
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
37-
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
36+
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
3837
// CHECK-NEXT: ],
3938
// CHECK-NEXT: "name": "FW"
4039
// CHECK: },
@@ -45,7 +44,6 @@
4544
// CHECK: ],
4645
// CHECK-NEXT: "context-hash": "{{.*}}",
4746
// CHECK: "file-deps": [
48-
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
4947
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
5048
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h"
5149
// CHECK-NEXT: ],

‎clang/test/ClangScanDeps/modules-incomplete-umbrella.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ framework module FW_Private {
2626
[{
2727
"file": "DIR/from_tu.m",
2828
"directory": "DIR",
29-
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
29+
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
3030
}]
3131
//--- from_tu.m
3232
#include "FW/FW.h"
@@ -45,8 +45,7 @@ framework module FW_Private {
4545
// CHECK_TU-NEXT: "context-hash": "{{.*}}",
4646
// CHECK_TU-NEXT: "file-deps": [
4747
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
48-
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
49-
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
48+
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
5049
// CHECK_TU-NEXT: ],
5150
// CHECK_TU-NEXT: "name": "FW"
5251
// CHECK_TU-NEXT: },
@@ -57,7 +56,6 @@ framework module FW_Private {
5756
// CHECK_TU: ],
5857
// CHECK_TU-NEXT: "context-hash": "{{.*}}",
5958
// CHECK_TU-NEXT: "file-deps": [
60-
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
6159
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
6260
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
6361
// CHECK_TU-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"
@@ -102,7 +100,7 @@ framework module FW_Private {
102100
[{
103101
"file": "DIR/from_module.m",
104102
"directory": "DIR",
105-
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
103+
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
106104
}]
107105
//--- module.modulemap
108106
module Mod { header "Mod.h" }
@@ -125,8 +123,7 @@ module Mod { header "Mod.h" }
125123
// CHECK_MODULE-NEXT: "context-hash": "{{.*}}",
126124
// CHECK_MODULE-NEXT: "file-deps": [
127125
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
128-
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
129-
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
126+
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
130127
// CHECK_MODULE-NEXT: ],
131128
// CHECK_MODULE-NEXT: "name": "FW"
132129
// CHECK_MODULE-NEXT: },
@@ -137,7 +134,6 @@ module Mod { header "Mod.h" }
137134
// CHECK_MODULE: ],
138135
// CHECK_MODULE-NEXT: "context-hash": "{{.*}}",
139136
// CHECK_MODULE-NEXT: "file-deps": [
140-
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
141137
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
142138
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
143139
// CHECK_MODULE-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"

‎clang/test/ClangScanDeps/modules-redefinition.m

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// This test checks that we don't report non-affecting system module maps.
2+
// More specifically, we check that explicitly-specified module map file is not
3+
// being shadowed in explicit build by a module map file found during implicit
4+
// module map search.
5+
6+
// RUN: rm -rf %t
7+
// RUN: split-file %s %t
8+
9+
//--- tu.m
10+
@import first;
11+
12+
//--- zeroth/module.modulemap
13+
module X {}
14+
15+
//--- first/module.modulemap
16+
module first { header "first.h" }
17+
//--- first/first.h
18+
@import third;
19+
//--- second/module.modulemap
20+
module X {}
21+
//--- third/module.modulemap
22+
module third {}
23+
24+
//--- cdb.json.template
25+
[{
26+
"file": "DIR/tu.m",
27+
"directory": "DIR",
28+
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=X -fmodule-map-file=DIR/zeroth/module.modulemap -isystem DIR/first -isystem DIR/second -isystem DIR/third -c DIR/tu.m -o DIR/tu.o"
29+
}]
30+
31+
// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
32+
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
33+
// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
34+
// CHECK: {
35+
// CHECK-NEXT: "modules": [
36+
// CHECK-NEXT: {
37+
// CHECK-NEXT: "clang-module-deps": [
38+
// CHECK-NEXT: {
39+
// CHECK-NEXT: "context-hash": "{{.*}}",
40+
// CHECK-NEXT: "module-name": "third"
41+
// CHECK-NEXT: }
42+
// CHECK-NEXT: ],
43+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/first/module.modulemap",
44+
// CHECK-NEXT: "command-line": [
45+
// CHECK: ],
46+
// CHECK-NEXT: "context-hash": "{{.*}}",
47+
// CHECK-NEXT: "file-deps": [
48+
// CHECK-NEXT: [[PREFIX]]/first/first.h",
49+
// CHECK-NEXT: [[PREFIX]]/first/module.modulemap",
50+
// CHECK-NEXT: [[PREFIX]]/third/module.modulemap"
51+
// CHECK-NEXT: ],
52+
// CHECK-NEXT: "name": "first"
53+
// CHECK-NEXT: }
54+
// CHECK: ]
55+
// CHECK: }
56+
57+
// RUN: %deps-to-rsp %t/result.json --module-name=third > %t/third.cc1.rsp
58+
// RUN: %deps-to-rsp %t/result.json --module-name=first > %t/first.cc1.rsp
59+
// RUN: %clang @%t/third.cc1.rsp
60+
// RUN: %clang @%t/first.cc1.rsp

0 commit comments

Comments
 (0)