Merge pull request #37981 from xedin/record-explored-types-incrementally

[ConstraintSystem] Record produced types incrementally

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -5369,6 +5369,9 @@ class TypeVariableBinding {
   TypeVariableBinding(TypeVariableType *typeVar, PotentialBinding &binding)
       : TypeVar(typeVar), Binding(binding) {}
 
+  TypeVariableType *getTypeVariable() const { return TypeVar; }
+  Type getType() const { return Binding.BindingType; }
+
   bool isDefaultable() const { return Binding.isDefaultableBinding(); }
 
   bool hasDefaultedProtocol() const {
@@ -5453,7 +5456,21 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
     if (needsToComputeNext() && !computeNext())
       return None;
 
-    return TypeVariableBinding(TypeVar, Bindings[Index++]);
+    auto &binding = Bindings[Index++];
+
+    // Record produced type as bound/explored early, otherwise
+    // it could be possible to re-discover it during `computeNext()`,
+    // which leads to duplicate bindings e.g. inferring fallback
+    // `Void` for a closure result type when `Void` was already
+    // inferred as a direct/transitive binding.
+    {
+      auto type = binding.BindingType;
+
+      BoundTypes.insert(type.getPointer());
+      ExploredTypes.insert(type->getCanonicalType());
+    }
+
+    return TypeVariableBinding(TypeVar, binding);
   }
 
   bool needsToComputeNext() const override { return Index >= Bindings.size(); }

lib/Sema/CSBindings.cpp

@@ -1705,10 +1705,6 @@ bool TypeVarBindingProducer::computeNext() {
     const auto type = binding.BindingType;
     assert(!type->hasError());
 
-    // After our first pass, note that we've explored these types.
-    if (NumTries == 0)
-      ExploredTypes.insert(type->getCanonicalType());
-
     // If we have a protocol with a default type, look for alternative
     // types to the default.
     if (NumTries == 0 && binding.hasDefaultedLiteralProtocol()) {

unittests/Sema/BindingInferenceTests.cpp

@@ -336,3 +336,62 @@ TEST_F(SemaTest, TestTransitiveProtocolInferenceThroughEquivalenceChains) {
   verifyProtocolInferenceResults(*bindings.TransitiveProtocols,
                                  {protocolTy0, protocolTy1});
 }
+
+TEST_F(SemaTest, TestNoDoubleVoidClosureResultInference) {
+  ConstraintSystemOptions options;
+  ConstraintSystem cs(DC, options);
+
+  auto verifyInference = [&](TypeVariableType *typeVar, unsigned numExpected) {
+    auto bindings = cs.getBindingsFor(typeVar);
+    TypeVarBindingProducer producer(bindings);
+
+    llvm::SmallPtrSet<Type, 2> inferredTypes;
+
+    while (auto binding = producer()) {
+      ASSERT_TRUE(binding.hasValue());
+      ASSERT_EQ(binding->getTypeVariable(), typeVar);
+      ASSERT_TRUE(inferredTypes.insert(binding->getType()).second);
+    }
+
+    ASSERT_EQ(inferredTypes.size(), numExpected);
+  };
+
+  auto *closureResultLoc =
+      cs.getConstraintLocator({}, ConstraintLocator::ClosureResult);
+
+  auto *closureResult = cs.createTypeVariable(closureResultLoc, /*options=*/0);
+
+  cs.addConstraint(ConstraintKind::Subtype, getStdlibType("Int"), closureResult,
+                   closureResultLoc);
+  cs.addConstraint(ConstraintKind::Subtype, closureResult, getStdlibType("Void"),
+                   closureResultLoc);
+
+  verifyInference(closureResult, 2);
+
+  auto closureResultWithTransitiveVoid = cs.createTypeVariable(closureResultLoc,
+                                                               /*options=*/0);
+
+  auto contextualVar = cs.createTypeVariable({}, /*options=*/0);
+
+  cs.addConstraint(ConstraintKind::Subtype, getStdlibType("Void"),
+                   contextualVar, cs.getConstraintLocator({}));
+
+  cs.addConstraint(ConstraintKind::Subtype, contextualVar,
+                   closureResultWithTransitiveVoid, closureResultLoc);
+
+  cs.addConstraint(ConstraintKind::Subtype, getStdlibType("Int"),
+                   closureResultWithTransitiveVoid, closureResultLoc);
+
+  verifyInference(closureResultWithTransitiveVoid, 2);
+
+  auto closureResultWithoutVoid =
+      cs.createTypeVariable(closureResultLoc, /*options=*/0);
+
+  // Supertype triggers `Void` inference
+  cs.addConstraint(ConstraintKind::Subtype, getStdlibType("Int"),
+                   closureResultWithoutVoid, closureResultLoc);
+  cs.addConstraint(ConstraintKind::Subtype, closureResultWithoutVoid,
+                   getStdlibType("String"), closureResultLoc);
+
+  verifyInference(closureResultWithoutVoid, 3);
+}