Skip to content

Commit 7144dab

Browse files
author
Nathan Hawes
committed
[ParseableInterface] Don't report out-of-date dependencies to the parent dependency tracker
We previously added dependencies to the tracker inline while validating a cached module's dependencies were up to date. If one of its dependencies ended up being out of date though, we shouldn't have added the previous dependencies, as that means the dependency list itself was also out of date. This patch changes the behavior to only add the module's dependencies once we've verified they're all up to date.
1 parent 58d622d commit 7144dab

File tree

2 files changed

+41
-5
lines changed

2 files changed

+41
-5
lines changed

lib/Frontend/ParseableInterfaceModuleLoader.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -726,8 +726,6 @@ class ParseableInterfaceModuleLoaderImpl {
726726
SmallString<128> SDKRelativeBuffer;
727727
for (auto &in : deps) {
728728
StringRef fullPath = getFullDependencyPath(in, SDKRelativeBuffer);
729-
if (dependencyTracker)
730-
dependencyTracker->addDependency(fullPath, /*isSystem*/in.isSDKRelative());
731729
if (!dependencyIsUpToDate(in, fullPath)) {
732730
LLVM_DEBUG(llvm::dbgs() << "Dep " << fullPath
733731
<< " is directly out of date\n");
@@ -950,7 +948,9 @@ class ParseableInterfaceModuleLoaderImpl {
950948
}
951949

952950
/// Looks up the best module to load for a given interface, and returns a
953-
/// buffer of the module's contents. See the main comment in
951+
/// buffer of the module's contents. Also reports the module's dependencies
952+
/// to the parent \c dependencyTracker if it came from the cache, or was built
953+
/// from the given interface. See the main comment in
954954
/// \c ParseableInterfaceModuleLoader.h for an explanation of the module
955955
/// loading strategy.
956956
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
@@ -998,6 +998,15 @@ class ParseableInterfaceModuleLoaderImpl {
998998
if (writeForwardingModule(module, cachedOutputPath, allDeps))
999999
return std::make_error_code(std::errc::not_supported);
10001000

1001+
// Report the module's dependencies to the dependencyTracker
1002+
if (dependencyTracker) {
1003+
SmallString<128> SDKRelativeBuffer;
1004+
for (auto &dep: allDeps) {
1005+
StringRef fullPath = getFullDependencyPath(dep, SDKRelativeBuffer);
1006+
dependencyTracker->addDependency(fullPath, dep.isSDKRelative());
1007+
}
1008+
}
1009+
10011010
return std::move(module.moduleBuffer);
10021011
}
10031012

test/ParseableInterface/ModuleCache/SDKDependencies.swift

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,16 @@
4747
// RUN: llvm-bcanalyzer -dump %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
4848
//
4949
// Check we didn't emit anything from the cache in the .d file either
50+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE-NEGATIVE
5051
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
5152
//
52-
// DEPFILE-NOT: /MCP/
53-
// DEPFILE-NOT: /prebuilt-cache/
53+
// DEPFILE-NEGATIVE-NOT: /MCP/
54+
// DEPFILE-NEGATIVE-NOT: /prebuilt-cache/
55+
//
56+
// DEPFILE-DAG: SomeCModule.h
57+
// DEPFILE-DAG: SdkLib.swiftinterface
58+
// DEPFILE-DAG: ExportedLib.swiftinterface
59+
// DEPFILE-DAG: SDKDependencies.swift
5460
//
5561
// RUN: %empty-directory(%t/MCP)
5662
// RUN: echo '2: PASSED'
@@ -90,6 +96,7 @@
9096
// NOCACHE-NOT: /MCP/
9197
//
9298
// Check we didn't emit anything from the cache in the .d file either
99+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE-NEGATIVE
93100
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
94101
//
95102
// RUN: %empty-directory(%t/MCP)
@@ -126,6 +133,7 @@
126133
// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefix=NOCACHE -DLIB_NAME=SdkLib
127134
//
128135
// Check we didn't emit anything from the cache in the .d file either
136+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE-NEGATIVE
129137
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
130138
//
131139
// RUN: %empty-directory(%t/MCP)
@@ -170,10 +178,29 @@
170178
// RUN: llvm-bcanalyzer -dump %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
171179
//
172180
// Check we didn't emit anything from the cache in the .d file either
181+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE-NEGATIVE
173182
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
174183
//
175184
// RUN: echo '5: PASSED'
176185

186+
187+
// 6) Keeping the existing cache, update ExportedLib to no longer depend on SomeCModule
188+
//
189+
// RUN: grep -v import %t/my-new-sdk/ExportedLib.swiftinterface > %t/my-new-sdk/ExportedLib.swiftinterface.updated
190+
// RUN: mv %t/my-new-sdk/ExportedLib.swiftinterface.updated %t/my-new-sdk/ExportedLib.swiftinterface
191+
//
192+
// RUN: sed -e 's/x > 3/5 > 3/g' %s | %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/new-dir/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies -
193+
//
194+
// Check we don't have SomeCModule listed in dependencies
195+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefixes=DEPFILE-NEGATIVE,DEPCHANGE-NEGATIVE
196+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPCHANGE
197+
//
198+
// DEPCHANGE-NEGATIVE-NOT: SomeCModule.h
199+
//
200+
// DEPCHANGE-DAG: SdkLib.swiftinterface
201+
// DEPCHANGE-DAG: ExportedLib.swiftinterface
202+
// DEPCHANGE-DAG: SDKDependencies.swift
203+
177204
import SdkLib
178205

179206
func foo() -> ExportedInterface {

0 commit comments

Comments
 (0)