Skip to content

Commit 94ae5f9

Browse files
jansvoboda11jansvoboda11
authored
[clang][modules][deps] Implicit modules are out of date when their explicit imports are (#138920)
The dependency scanner mixes implicitly- and explicitly-built modules. When an implicitly-built module imports an explicitly-built one, we never run the modification time validation checks, resulting in an out-of-date module cache. This PR fixes that by only skipping the modification time validation checks when both the imported module and its importer are built explicitly. rdar://150230022
1 parent a6385a8 commit 94ae5f9

File tree

2 files changed

+183
-3
lines changed

2 files changed

+183
-3
lines changed

‎clang/lib/Serialization/ModuleManager.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,12 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
110110
// Look for the file entry. This only fails if the expected size or
111111
// modification time differ.
112112
OptionalFileEntryRef Entry;
113-
const bool IgnoreModTime =
114-
(Type == MK_ExplicitModule || Type == MK_PrebuiltModule);
113+
bool IgnoreModTime = Type == MK_ExplicitModule || Type == MK_PrebuiltModule;
114+
if (ImportedBy)
115+
IgnoreModTime &= ImportedBy->Kind == MK_ExplicitModule ||
116+
ImportedBy->Kind == MK_PrebuiltModule;
115117
if (IgnoreModTime) {
116-
// If we're not expecting to pull this file out of the module cache, it
118+
// If neither this file nor the importer are in the module cache, this file
117119
// might have a different mtime due to being moved across filesystems in
118120
// a distributed build. The size must still match, though. (As must the
119121
// contents, but we can't check that.)

‎clang/test/ClangScanDeps/modules-pch-common-stale.c

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Test that modifications to a common header (imported from both a PCH and a TU)
2+
// cause rebuilds of dependent modules imported from the TU on incremental build.
3+
4+
// RUN: rm -rf %t
5+
// RUN: split-file %s %t
6+
7+
//--- module.modulemap
8+
module mod_common { header "mod_common.h" }
9+
module mod_tu { header "mod_tu.h" }
10+
module mod_tu_extra { header "mod_tu_extra.h" }
11+
12+
//--- mod_common.h
13+
#define MOD_COMMON_MACRO 0
14+
//--- mod_tu.h
15+
#include "mod_common.h"
16+
#if MOD_COMMON_MACRO
17+
#include "mod_tu_extra.h"
18+
#endif
19+
20+
//--- mod_tu_extra.h
21+
22+
//--- prefix.h
23+
#include "mod_common.h"
24+
25+
//--- tu.c
26+
#include "mod_tu.h"
27+
28+
// Clean: scan the PCH.
29+
// RUN: clang-scan-deps -format experimental-full -o %t/deps_pch_clean.json -- \
30+
// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch -F %t \
31+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
32+
33+
// Clean: build the PCH.
34+
// RUN: %deps-to-rsp %t/deps_pch_clean.json --module-name mod_common > %t/mod_common.rsp
35+
// RUN: %deps-to-rsp %t/deps_pch_clean.json --tu-index 0 > %t/pch.rsp
36+
// RUN: %clang @%t/mod_common.rsp
37+
// RUN: %clang @%t/pch.rsp
38+
39+
// Clean: scan the TU.
40+
// RUN: clang-scan-deps -format experimental-full -o %t/deps_tu_clean.json -- \
41+
// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h -F %t \
42+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
43+
// RUN: cat %t/deps_tu_clean.json | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-TU-CLEAN -DPREFIX=%/t
44+
// CHECK-TU-CLEAN: {
45+
// CHECK-TU-CLEAN-NEXT: "modules": [
46+
// CHECK-TU-CLEAN-NEXT: {
47+
// CHECK-TU-CLEAN-NEXT: "clang-module-deps": [],
48+
// CHECK-TU-CLEAN-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
49+
// CHECK-TU-CLEAN-NEXT: "command-line": [
50+
// CHECK-TU-CLEAN: ],
51+
// CHECK-TU-CLEAN-NEXT: "context-hash": "{{.*}}",
52+
// CHECK-TU-CLEAN-NEXT: "file-deps": [
53+
// CHECK-TU-CLEAN-NEXT: "[[PREFIX]]/module.modulemap",
54+
// CHECK-TU-CLEAN-NEXT: "[[PREFIX]]/mod_tu.h"
55+
// CHECK-TU-CLEAN-NEXT: ],
56+
// CHECK-TU-CLEAN-NEXT: "link-libraries": [],
57+
// CHECK-TU-CLEAN-NEXT: "name": "mod_tu"
58+
// CHECK-TU-CLEAN-NEXT: }
59+
// CHECK-TU-CLEAN-NEXT: ],
60+
// CHECK-TU-CLEAN-NEXT: "translation-units": [
61+
// CHECK-TU-CLEAN-NEXT: {
62+
// CHECK-TU-CLEAN-NEXT: "commands": [
63+
// CHECK-TU-CLEAN-NEXT: {
64+
// CHECK-TU-CLEAN-NEXT: "clang-context-hash": "{{.*}}",
65+
// CHECK-TU-CLEAN-NEXT: "clang-module-deps": [
66+
// CHECK-TU-CLEAN-NEXT: {
67+
// CHECK-TU-CLEAN-NEXT: "context-hash": "{{.*}}",
68+
// CHECK-TU-CLEAN-NEXT: "module-name": "mod_tu"
69+
// CHECK-TU-CLEAN-NEXT: }
70+
// CHECK-TU-CLEAN-NEXT: ],
71+
// CHECK-TU-CLEAN-NEXT: "command-line": [
72+
// CHECK-TU-CLEAN: ],
73+
// CHECK-TU-CLEAN-NEXT: "executable": "{{.*}}",
74+
// CHECK-TU-CLEAN-NEXT: "file-deps": [
75+
// CHECK-TU-CLEAN-NEXT: "[[PREFIX]]/tu.c",
76+
// CHECK-TU-CLEAN-NEXT: "[[PREFIX]]/prefix.h.pch"
77+
// CHECK-TU-CLEAN-NEXT: ],
78+
// CHECK-TU-CLEAN-NEXT: "input-file": "[[PREFIX]]/tu.c"
79+
// CHECK-TU-CLEAN-NEXT: }
80+
// CHECK-TU-CLEAN-NEXT: ]
81+
// CHECK-TU-CLEAN-NEXT: }
82+
// CHECK-TU-CLEAN: ]
83+
// CHECK-TU-CLEAN: }
84+
85+
// Clean: build the TU.
86+
// RUN: %deps-to-rsp %t/deps_tu_clean.json --module-name mod_tu > %t/mod_tu.rsp
87+
// RUN: %deps-to-rsp %t/deps_tu_clean.json --tu-index 0 > %t/tu.rsp
88+
// RUN: %clang @%t/mod_tu.rsp
89+
// RUN: %clang @%t/tu.rsp
90+
91+
// Incremental: modify the common module.
92+
// RUN: sleep 1
93+
// RUN: echo "#define MOD_COMMON_MACRO 1" > %t/mod_common.h
94+
95+
// Incremental: scan the PCH.
96+
// RUN: clang-scan-deps -format experimental-full -o %t/deps_pch_incremental.json -- \
97+
// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch -F %t \
98+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
99+
100+
// Incremental: build the PCH.
101+
// RUN: %deps-to-rsp %t/deps_pch_incremental.json --module-name mod_common > %t/mod_common.rsp
102+
// RUN: %deps-to-rsp %t/deps_pch_incremental.json --tu-index 0 > %t/pch.rsp
103+
// RUN: %clang @%t/mod_common.rsp
104+
// RUN: %clang @%t/pch.rsp
105+
106+
// Incremental: scan the TU. This needs to invalidate modules imported from the
107+
// TU that depend on modules imported from the PCH and discover the
108+
// new dependency on 'mod_tu_extra'.
109+
// RUN: clang-scan-deps -format experimental-full -o %t/deps_tu_incremental.json -- \
110+
// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h -F %t \
111+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
112+
// RUN: cat %t/deps_tu_incremental.json | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-TU-INCREMENTAL -DPREFIX=%/t
113+
// CHECK-TU-INCREMENTAL: {
114+
// CHECK-TU-INCREMENTAL-NEXT: "modules": [
115+
// CHECK-TU-INCREMENTAL-NEXT: {
116+
// CHECK-TU-INCREMENTAL-NEXT: "clang-module-deps": [
117+
// CHECK-TU-INCREMENTAL-NEXT: {
118+
// CHECK-TU-INCREMENTAL-NEXT: "context-hash": "{{.*}}",
119+
// CHECK-TU-INCREMENTAL-NEXT: "module-name": "mod_tu_extra"
120+
// CHECK-TU-INCREMENTAL-NEXT: }
121+
// CHECK-TU-INCREMENTAL-NEXT: ],
122+
// CHECK-TU-INCREMENTAL-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
123+
// CHECK-TU-INCREMENTAL-NEXT: "command-line": [
124+
// CHECK-TU-INCREMENTAL: ],
125+
// CHECK-TU-INCREMENTAL-NEXT: "context-hash": "{{.*}}",
126+
// CHECK-TU-INCREMENTAL-NEXT: "file-deps": [
127+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/module.modulemap",
128+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/mod_tu.h"
129+
// CHECK-TU-INCREMENTAL-NEXT: ],
130+
// CHECK-TU-INCREMENTAL-NEXT: "link-libraries": [],
131+
// CHECK-TU-INCREMENTAL-NEXT: "name": "mod_tu"
132+
// CHECK-TU-INCREMENTAL-NEXT: },
133+
// CHECK-TU-INCREMENTAL-NEXT: {
134+
// CHECK-TU-INCREMENTAL-NEXT: "clang-module-deps": [],
135+
// CHECK-TU-INCREMENTAL-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
136+
// CHECK-TU-INCREMENTAL-NEXT: "command-line": [
137+
// CHECK-TU-INCREMENTAL: ],
138+
// CHECK-TU-INCREMENTAL-NEXT: "context-hash": "{{.*}}",
139+
// CHECK-TU-INCREMENTAL-NEXT: "file-deps": [
140+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/module.modulemap",
141+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/mod_tu_extra.h"
142+
// CHECK-TU-INCREMENTAL-NEXT: ],
143+
// CHECK-TU-INCREMENTAL-NEXT: "link-libraries": [],
144+
// CHECK-TU-INCREMENTAL-NEXT: "name": "mod_tu_extra"
145+
// CHECK-TU-INCREMENTAL-NEXT: }
146+
// CHECK-TU-INCREMENTAL-NEXT: ],
147+
// CHECK-TU-INCREMENTAL-NEXT: "translation-units": [
148+
// CHECK-TU-INCREMENTAL-NEXT: {
149+
// CHECK-TU-INCREMENTAL-NEXT: "commands": [
150+
// CHECK-TU-INCREMENTAL-NEXT: {
151+
// CHECK-TU-INCREMENTAL-NEXT: "clang-context-hash": "{{.*}}",
152+
// CHECK-TU-INCREMENTAL-NEXT: "clang-module-deps": [
153+
// CHECK-TU-INCREMENTAL-NEXT: {
154+
// CHECK-TU-INCREMENTAL-NEXT: "context-hash": "{{.*}}",
155+
// CHECK-TU-INCREMENTAL-NEXT: "module-name": "mod_tu"
156+
// CHECK-TU-INCREMENTAL-NEXT: }
157+
// CHECK-TU-INCREMENTAL-NEXT: ],
158+
// CHECK-TU-INCREMENTAL-NEXT: "command-line": [
159+
// CHECK-TU-INCREMENTAL: ],
160+
// CHECK-TU-INCREMENTAL-NEXT: "executable": "{{.*}}",
161+
// CHECK-TU-INCREMENTAL-NEXT: "file-deps": [
162+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/tu.c",
163+
// CHECK-TU-INCREMENTAL-NEXT: "[[PREFIX]]/prefix.h.pch"
164+
// CHECK-TU-INCREMENTAL-NEXT: ],
165+
// CHECK-TU-INCREMENTAL-NEXT: "input-file": "[[PREFIX]]/tu.c"
166+
// CHECK-TU-INCREMENTAL-NEXT: }
167+
// CHECK-TU-INCREMENTAL-NEXT: ]
168+
// CHECK-TU-INCREMENTAL-NEXT: }
169+
// CHECK-TU-INCREMENTAL: ]
170+
// CHECK-TU-INCREMENTAL: }
171+
172+
// Incremental: build the TU.
173+
// RUN: %deps-to-rsp %t/deps_tu_incremental.json --module-name mod_tu_extra > %t/mod_tu_extra.rsp
174+
// RUN: %deps-to-rsp %t/deps_tu_incremental.json --module-name mod_tu > %t/mod_tu.rsp
175+
// RUN: %deps-to-rsp %t/deps_tu_incremental.json --tu-index 0 > %t/tu.rsp
176+
// RUN: %clang @%t/mod_tu_extra.rsp
177+
// RUN: %clang @%t/mod_tu.rsp
178+
// RUN: %clang @%t/tu.rsp

0 commit comments

Comments
 (0)