Skip to content

Commit 932a91e

Browse files
author
David Ungar
authored
Merge pull request #32219 from davidungar/quick-dependency-bug-fix
Simplest fix to a dependency bug.
2 parents 3e280b8 + da14e31 commit 932a91e

File tree

6 files changed

+25
-28
lines changed

6 files changed

+25
-28
lines changed

lib/AST/AbstractSourceFileDepGraphFactory.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,30 @@ void AbstractSourceFileDepGraphFactory::addAUsedDecl(
7979
const DependencyKey &defKey, const DependencyKey &useKey) {
8080
auto *defNode =
8181
g.findExistingNodeOrCreateIfNew(defKey, None, false /* = !isProvides */);
82+
8283
// If the depended-upon node is defined in this file, then don't
8384
// create an arc to the user, when the user is the whole file.
8485
// Otherwise, if the defNode's type-body fingerprint changes,
8586
// the whole file will be marked as dirty, losing the benefit of the
8687
// fingerprint.
87-
if (defNode->getIsProvides() &&
88-
useKey.getKind() == NodeKind::sourceFileProvide)
89-
return;
88+
89+
// if (defNode->getIsProvides() &&
90+
// useKey.getKind() == NodeKind::sourceFileProvide)
91+
// return;
92+
93+
// Turns out the above three lines cause miscompiles, so comment them out
94+
// for now. We might want them back if we can change the inputs to this
95+
// function to be more precise.
96+
97+
// Example of a miscompile:
98+
// In main.swift
99+
// func foo(_: Any) { print("Hello Any") }
100+
// foo(123)
101+
// Then add the following line to another file:
102+
// func foo(_: Int) { print("Hello Int") }
103+
// Although main.swift needs to get recompiled, the commented-out code below
104+
// prevents that.
105+
90106
auto nullableUse = g.findExistingNode(useKey);
91107
assert(nullableUse.isNonNull() && "Use must be an already-added provides");
92108
auto *useNode = nullableUse.get();

test/Frontend/Fingerprints/class-fingerprint.swift

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,4 @@
7171

7272
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7373

74-
// RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
75-
76-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
77-
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
78-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
79-
74+
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4

test/Frontend/Fingerprints/enum-fingerprint.swift

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,4 @@
7171

7272
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7373

74-
// RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
75-
76-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
77-
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
78-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
79-
74+
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4

test/Frontend/Fingerprints/protocol-fingerprint.swift

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,5 @@
7171

7272
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7373

74-
// RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
75-
76-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
77-
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
78-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
74+
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
7975

test/Frontend/Fingerprints/struct-fingerprint.swift

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,4 @@
7171

7272
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7373

74-
// RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
75-
76-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
77-
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
78-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
79-
74+
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4

unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -807,10 +807,10 @@ TEST(ModuleDepGraph, UseFingerprints) {
807807
{
808808
const auto jobs =
809809
simulateReload(graph, &job0, {{NodeKind::nominal, {"A1@11", "A2@2"}}});
810-
EXPECT_EQ(2u, jobs.size());
810+
EXPECT_EQ(3u, jobs.size());
811811
EXPECT_TRUE(contains(jobs, &job0));
812812
EXPECT_TRUE(contains(jobs, &job1));
813-
EXPECT_FALSE(contains(jobs, &job2));
813+
EXPECT_TRUE(contains(jobs, &job2));
814814
EXPECT_FALSE(contains(jobs, &job3));
815815
}
816816
}

0 commit comments

Comments
 (0)