Merge pull request swiftlang#30123 from davidungar/fix-overly-conserv…

…ative-fingerprint-bug [Incremental] Fix overly conservative fingerprint bug & fix tests.
daeken · Feb 28, 2020 · b860356 · b860356
2 parents c0b9394 + d09c906
commit b860356
Show file tree

Hide file tree

Showing 12 changed files with 62 additions and 57 deletions.
diff --git a/lib/AST/AbstractSourceFileDepGraphFactory.cpp b/lib/AST/AbstractSourceFileDepGraphFactory.cpp
@@ -79,6 +79,14 @@ void AbstractSourceFileDepGraphFactory::addAUsedDecl(
     const DependencyKey &defKey, const DependencyKey &useKey) {
   auto *defNode =
       g.findExistingNodeOrCreateIfNew(defKey, None, false /* = !isProvides */);
+  // If the depended-upon node is defined in this file, then don't
+  // create an arc to the user, when the user is the whole file.
+  // Otherwise, if the defNode's type-body fingerprint changes,
+  // the whole file will be marked as dirty, losing the benefit of the
+  // fingerprint.
+  if (defNode->getIsProvides() &&
+      useKey.getKind() == NodeKind::sourceFileProvide)
+    return;
   auto nullableUse = g.findExistingNode(useKey);
   assert(nullableUse.isNonNull() && "Use must be an already-added provides");
   auto *useNode = nullableUse.get();

diff --git a/test/Frontend/Inputs/type-fingerprint/a.swift b/test/Frontend/Inputs/type-fingerprint/a.swift
diff --git a/test/Frontend/Inputs/type-fingerprint/b0.swift b/test/Frontend/Inputs/type-fingerprint/b0.swift
diff --git a/test/Frontend/Inputs/type-fingerprint/b1.swift b/test/Frontend/Inputs/type-fingerprint/b1.swift
diff --git a/test/Frontend/Inputs/type-fingerprint/definesAB-after.swift b/test/Frontend/Inputs/type-fingerprint/definesAB-after.swift
@@ -0,0 +1,5 @@
+struct A {
+  var x = 17
+}
+struct B {
+}
diff --git a/test/Frontend/Inputs/type-fingerprint/definesAB-before.swift b/test/Frontend/Inputs/type-fingerprint/definesAB-before.swift
@@ -0,0 +1,4 @@
+struct A {
+}
+struct B {
+}
diff --git a/test/Frontend/Inputs/type-fingerprint/main.swift b/test/Frontend/Inputs/type-fingerprint/main.swift
@@ -1,3 +1 @@
-func foo() {
-  B1()
-}
+
diff --git a/test/Frontend/Inputs/type-fingerprint/ofm.json b/test/Frontend/Inputs/type-fingerprint/ofm.json
@@ -1,17 +1,22 @@
 {
-  "./a.swift": {
-    "object": "./a.o",
-    "swift-dependencies": "./a.swiftdeps"
-  },
-  "./b.swift": {
-    "object": "./b.o",
-    "swift-dependencies": "./b.swiftdeps"
-  },
-  "./main.swift": {
+  "main.swift": {
     "object": "./main.o",
     "swift-dependencies": "./main.swiftdeps"
   },
+  "definesAB.swift": {
+    "object": "./definesAB.o",
+    "swift-dependencies": "./definesAB.swiftdeps"
+  },
+  "usesA.swift": {
+    "object": "./usesA.o",
+    "swift-dependencies": "./usesA.swiftdeps"
+  },
+  "usesB.swift": {
+    "object": "./usesB.o",
+    "swift-dependencies": "./usesB.swiftdeps"
+  },
   "": {
     "swift-dependencies": "./main~buildrecord.swiftdeps"
   }
 }
+
diff --git a/test/Frontend/Inputs/type-fingerprint/usesA.swift b/test/Frontend/Inputs/type-fingerprint/usesA.swift
@@ -0,0 +1 @@
+let a = A()
diff --git a/test/Frontend/Inputs/type-fingerprint/usesB.swift b/test/Frontend/Inputs/type-fingerprint/usesB.swift
@@ -0,0 +1 @@
+let b = B()
diff --git a/test/Frontend/type-fingerprint.swift b/test/Frontend/type-fingerprint.swift
@@ -8,38 +8,37 @@
 
 // Establish status quo
 
+
 // RUN: %empty-directory(%t)
-// RUN: cp %S/Inputs/type-fingerprint/{main,a}.swift %t
-// RUN: cp %S/Inputs/type-fingerprint/ofm.json %t
-// RUN: cp %S/Inputs/type-fingerprint/b0.swift %t/b.swift
+// RUN: cp %S/Inputs/type-fingerprint/* %t
+// RUN: cp %t/definesAB{-before,}.swift
 
 // Seeing weird failure on CI, so set the mod times
 // RUN: touch -t 200101010101 %t/*.swift
 
-// RUN: cd %t && %swiftc_driver  -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output1
+// RUN: cd %t && %swiftc_driver  -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output1
 
-// only-run-for-debugging: cp %t/b.swiftdeps %t/b1.swiftdeps
+// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
 
 // Change one type, but uses of all types get recompiled
 
-// RUN: cp %S/Inputs/type-fingerprint/b1.swift %t/b.swift
+// RUN: cp %t/definesAB{-after,}.swift
 
-// Seeing weird failure on CI, so ensure that b.swift is newer
+// Seeing weird failure on CI, so ensure that definesAB.swift is newer
 // RUN: touch -t 200201010101 %t/*
 // RUN: touch -t 200101010101 %t/*.swift
-// RUN: touch -t 200301010101 %t/b.swift
+// RUN: touch -t 200301010101 %t/definesAB.swift
 
-// RUN: cd %t && %swiftc_driver  -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output2
+// RUN: cd %t && %swiftc_driver  -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift  -module-name main -output-file-map ofm.json >&output2
 
 // Save for debugging:
-// only-run-for-debugging: cp %t/b.swiftdeps %t/b2.swiftdeps
+// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
 
 // RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output2
 
-// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: b.o <= b.swift}
-// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: main.o <= main.swift}
-// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: a.o <= a.swift}
-
+// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
+// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
+// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
 
 // =============================================================================
 // With the fingerprints
@@ -48,33 +47,33 @@
 // Establish status quo
 
 // RUN: %empty-directory(%t)
-// RUN: cp %S/Inputs/type-fingerprint/{main,a}.swift %t
-// RUN: cp %S/Inputs/type-fingerprint/ofm.json %t
-// RUN: cp %S/Inputs/type-fingerprint/b0.swift %t/b.swift
+// RUN: cp %S/Inputs/type-fingerprint/* %t
+// RUN: cp %t/definesAB{-before,}.swift
 
 // Seeing weird failure on CI, so set the mod times
 // RUN: touch -t 200101010101 %t/*.swift
 
-// RUN: cd %t && %swiftc_driver  -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output3
+// RUN: cd %t && %swiftc_driver  -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output3
+
+// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB3.swiftdeps
 
-// only-run-for-debugging: cp %t/b.swiftdeps %t/b3.swiftdeps
 
 // Change one type, only uses of that type get recompiled
 
-// RUN: cp %S/Inputs/type-fingerprint/b1.swift %t/b.swift
+// RUN: cp %t/definesAB{-after,}.swift
 
-// Seeing weird failure on CI, so ensure that b.swift is newer
+// Seeing weird failure on CI, so ensure that definesAB.swift is newer
 // RUN: touch -t 200201010101 %t/*
 // RUN: touch -t 200101010101 %t/*.swift
-// RUN: touch -t 200301010101 %t/b.swift
+// RUN: touch -t 200301010101 %t/definesAB.swift
 
-// RUN: cd %t && %swiftc_driver  -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output4
+// RUN: cd %t && %swiftc_driver  -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output4
 
-// only-run-for-debugging: cp %t/b.swiftdeps %t/b4.swiftdeps
+// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
 
 // RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
 
-// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: a.o <= a.swift}
-// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: main.o <= main.swift}
-// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: // CHECK-MAINB-RECOMPILED: a.o <= a.swift}
+// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
+// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
+// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
 
diff --git a/unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp b/unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp
@@ -798,8 +798,9 @@ TEST(ModuleDepGraph, UseFingerprints) {
 
   // Because when A1 changes, B1 and not B2 is affected, only jobs1 and job2
   // should be recompiled, except type fingerprints is off!
+  // Include a dependency on A1, to ensure it does not muck things up.
 
-  simulateLoad(graph, &job0, {{NodeKind::nominal, {"A1@1", "A2@2"}}});
+  simulateLoad(graph, &job0, {{NodeKind::nominal, {"A1@1", "A2@2", "A1->"}}});
   simulateLoad(graph, &job1, {{NodeKind::nominal, {"B1", "A1->"}}});
   simulateLoad(graph, &job2, {{NodeKind::nominal, {"C1", "A2->"}}});
   simulateLoad(graph, &job3, {{NodeKind::nominal, {"D1"}}});