swiftwasm · pull · Jun 7, 2020 · May 30, 2020 · May 30, 2020 · May 31, 2020
diff --git a/lib/AST/AbstractSourceFileDepGraphFactory.cpp b/lib/AST/AbstractSourceFileDepGraphFactory.cpp
@@ -79,14 +79,30 @@ 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;
+
+  //  if (defNode->getIsProvides() &&
+  //      useKey.getKind() == NodeKind::sourceFileProvide)
+  //    return;
+
+  // Turns out the above three lines cause miscompiles, so comment them out
+  // for now. We might want them back if we can change the inputs to this
+  // function to be more precise.
+
+  // Example of a miscompile:
+  // In main.swift
+  // func foo(_: Any) { print("Hello Any") }
+  //    foo(123)
+  // Then add the following line to another file:
+  // func foo(_: Int) { print("Hello Int") }
+  // Although main.swift needs to get recompiled, the commented-out code below
+  // prevents that.
+
   auto nullableUse = g.findExistingNode(useKey);
   assert(nullableUse.isNonNull() && "Use must be an already-added provides");
   auto *useNode = nullableUse.get();

diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -955,6 +955,72 @@ TypeVariableType *ConstraintSystem::isRepresentativeFor(
   return *member;
 }
 
+static Optional<std::pair<VarDecl *, Type>>
+getPropertyWrapperInformationFromOverload(
+    SelectedOverload resolvedOverload, DeclContext *DC,
+    llvm::function_ref<Optional<std::pair<VarDecl *, Type>>(VarDecl *)>
+        getInformation) {
+  if (auto *decl =
+          dyn_cast_or_null<VarDecl>(resolvedOverload.choice.getDeclOrNull())) {
+    if (auto declInformation = getInformation(decl)) {
+      Type type;
+      VarDecl *memberDecl;
+      std::tie(memberDecl, type) = *declInformation;
+      if (Type baseType = resolvedOverload.choice.getBaseType()) {
+        type =
+            baseType->getTypeOfMember(DC->getParentModule(), memberDecl, type);
+      }
+      return std::make_pair(decl, type);
+    }
+  }
+  return None;
+}
+
+Optional<std::pair<VarDecl *, Type>>
+ConstraintSystem::getStorageWrapperInformation(
+    SelectedOverload resolvedOverload) {
+  return getPropertyWrapperInformationFromOverload(
+      resolvedOverload, DC,
+      [](VarDecl *decl) -> Optional<std::pair<VarDecl *, Type>> {
+        if (!decl->hasAttachedPropertyWrapper())
+          return None;
+
+        auto storageWrapper = decl->getPropertyWrapperStorageWrapper();
+        if (!storageWrapper)
+          return None;
+
+        return std::make_pair(storageWrapper,
+                              storageWrapper->getInterfaceType());
+      });
+}
+
+Optional<std::pair<VarDecl *, Type>>
+ConstraintSystem::getPropertyWrapperInformation(
+    SelectedOverload resolvedOverload) {
+  return getPropertyWrapperInformationFromOverload(
+      resolvedOverload, DC,
+      [](VarDecl *decl) -> Optional<std::pair<VarDecl *, Type>> {
+        if (!decl->hasAttachedPropertyWrapper())
+          return None;
+
+        return std::make_pair(decl,
+                              decl->getPropertyWrapperBackingPropertyType());
+      });
+}
+
+Optional<std::pair<VarDecl *, Type>>
+ConstraintSystem::getWrappedPropertyInformation(
+    SelectedOverload resolvedOverload) {
+  return getPropertyWrapperInformationFromOverload(
+      resolvedOverload, DC,
+      [](VarDecl *decl) -> Optional<std::pair<VarDecl *, Type>> {
+        if (auto wrapped = decl->getOriginalWrappedProperty())
+          return std::make_pair(decl, wrapped->getInterfaceType());
+
+        return None;
+      });
+}
+
 /// Does a var or subscript produce an l-value?
 ///
 /// \param baseType - the type of the base on which this object

diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h
@@ -3237,62 +3237,18 @@ class ConstraintSystem {
   /// Gets the VarDecl associateed with resolvedOverload, and the type of the
   /// storage wrapper if the decl has an associated storage wrapper.
   Optional<std::pair<VarDecl *, Type>>
-  getStorageWrapperInformation(SelectedOverload resolvedOverload) {
-    if (resolvedOverload.choice.isDecl()) {
-      if (auto *decl = dyn_cast<VarDecl>(resolvedOverload.choice.getDecl())) {
-        if (decl->hasAttachedPropertyWrapper()) {
-          if (auto storageWrapper = decl->getPropertyWrapperStorageWrapper()) {
-            Type type = storageWrapper->getInterfaceType();
-            if (Type baseType = resolvedOverload.choice.getBaseType()) {
-              type = baseType->getTypeOfMember(DC->getParentModule(),
-                                               storageWrapper, type);
-            }
-            return std::make_pair(decl, type);
-          }
-        }
-      }
-    }
-    return None;
-  }
+  getStorageWrapperInformation(SelectedOverload resolvedOverload);
 
   /// Gets the VarDecl associateed with resolvedOverload, and the type of the
   /// backing storage if the decl has an associated property wrapper.
   Optional<std::pair<VarDecl *, Type>>
-  getPropertyWrapperInformation(SelectedOverload resolvedOverload) {
-    if (resolvedOverload.choice.isDecl()) {
-      if (auto *decl = dyn_cast<VarDecl>(resolvedOverload.choice.getDecl())) {
-        if (decl->hasAttachedPropertyWrapper()) {
-          auto wrapperTy = decl->getPropertyWrapperBackingPropertyType();
-          if (Type baseType = resolvedOverload.choice.getBaseType()) {
-            wrapperTy = baseType->getTypeOfMember(DC->getParentModule(),
-                                                  decl, wrapperTy);
-          }
-          return std::make_pair(decl, wrapperTy);
-        }
-      }
-    }
-    return None;
-  }
+  getPropertyWrapperInformation(SelectedOverload resolvedOverload);
 
   /// Gets the VarDecl, and the type of the type property that it wraps if
   /// resolved overload has a decl which is the backing storage for a
   /// property wrapper.
   Optional<std::pair<VarDecl *, Type>>
-  getWrappedPropertyInformation(SelectedOverload resolvedOverload) {
-    if (resolvedOverload.choice.isDecl()) {
-      if (auto *decl = dyn_cast<VarDecl>(resolvedOverload.choice.getDecl())) {
-        if (auto wrapped = decl->getOriginalWrappedProperty()) {
-          Type type = wrapped->getInterfaceType();
-          if (Type baseType = resolvedOverload.choice.getBaseType()) {
-            type = baseType->getTypeOfMember(DC->getParentModule(),
-                                             wrapped, type);
-          }
-          return std::make_pair(decl, type);
-        }
-      }
-    }
-    return None;
-  }
+  getWrappedPropertyInformation(SelectedOverload resolvedOverload);
 
   /// Merge the equivalence sets of the two type variables.
   ///

diff --git a/test/Frontend/Fingerprints/class-fingerprint.swift b/test/Frontend/Fingerprints/class-fingerprint.swift
@@ -71,9 +71,4 @@
 
 // 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: 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}
-
+// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
diff --git a/test/Frontend/Fingerprints/enum-fingerprint.swift b/test/Frontend/Fingerprints/enum-fingerprint.swift
@@ -71,9 +71,4 @@
 
 // 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: 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}
-
+// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
diff --git a/test/Frontend/Fingerprints/protocol-fingerprint.swift b/test/Frontend/Fingerprints/protocol-fingerprint.swift
@@ -71,9 +71,5 @@
 
 // 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: 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}
+// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
 
diff --git a/test/Frontend/Fingerprints/struct-fingerprint.swift b/test/Frontend/Fingerprints/struct-fingerprint.swift
@@ -71,9 +71,4 @@
 
 // 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: 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}
-
+// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
diff --git a/unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp b/unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp
@@ -807,10 +807,10 @@ TEST(ModuleDepGraph, UseFingerprints) {
   {
     const auto jobs =
         simulateReload(graph, &job0, {{NodeKind::nominal, {"A1@11", "A2@2"}}});
-    EXPECT_EQ(2u, jobs.size());
+    EXPECT_EQ(3u, jobs.size());
     EXPECT_TRUE(contains(jobs, &job0));
     EXPECT_TRUE(contains(jobs, &job1));
-    EXPECT_FALSE(contains(jobs, &job2));
+    EXPECT_TRUE(contains(jobs, &job2));
     EXPECT_FALSE(contains(jobs, &job3));
   }
 }