Merge pull request swiftlang#35141 from xedin/adjust-protocol-coverage

[CSBindings] A couple of minor adjustments to inferring default bindings from literal protocols

Author: xedin

lib/Sema/CSBindings.cpp

@@ -401,12 +401,45 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
   // Key is a literal protocol requirement, Value indicates whether (first)
   // given protocol is a direct requirement, and (second) whether it has been
   // covered by an existing binding.
+  bool canBeNil = false;
   llvm::SmallMapVector<ProtocolDecl *, std::pair<bool, bool>, 4>
       literalProtocols;
   for (auto *constraint : Protocols) {
-    if (constraint->getKind() == ConstraintKind::LiteralConformsTo)
-      literalProtocols.insert({constraint->getProtocol(),
-                               {isDirectRequirement(constraint), false}});
+    if (constraint->getKind() != ConstraintKind::LiteralConformsTo)
+      continue;
+
+    auto *protocol = constraint->getProtocol();
+
+    // No reason to add `ExpressibleByNilLiteral` into the set
+    // because it doesn't have a default type.
+    if (protocol->isSpecificProtocol(
+            KnownProtocolKind::ExpressibleByNilLiteral)) {
+      canBeNil = true;
+      continue;
+    }
+
+    // Let's try to coalesce integer and floating point literal protocols
+    // if they appear together because the only possible default type that
+    // could satisfy both requirements is `Double`.
+    {
+      if (protocol->isSpecificProtocol(
+              KnownProtocolKind::ExpressibleByIntegerLiteral)) {
+        auto *floatLiteral = CS.getASTContext().getProtocol(
+            KnownProtocolKind::ExpressibleByFloatLiteral);
+        if (literalProtocols.count(floatLiteral))
+          continue;
+      }
+
+      if (protocol->isSpecificProtocol(
+              KnownProtocolKind::ExpressibleByFloatLiteral)) {
+        auto *intLiteral = CS.getASTContext().getProtocol(
+            KnownProtocolKind::ExpressibleByIntegerLiteral);
+        literalProtocols.erase(intLiteral);
+      }
+    }
+
+    literalProtocols.insert(
+        {protocol, {isDirectRequirement(constraint), false}});
   }
 
   for (auto &binding : Bindings) {
@@ -435,19 +468,20 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
       if (isCovered)
         continue;
 
-      // FIXME: This is a hack and it's incorrect because it depends
-      // on ordering of the literal procotols e.g. if `ExpressibleByNilLiteral`
-      // appears before e.g. `ExpressibleByIntegerLiteral` we'd drop
-      // optionality although that would be incorrect.
       do {
         // If the type conforms to this protocol, we're covered.
         if (TypeChecker::conformsToProtocol(type, protocol, cs.DC)) {
           isCovered = true;
           break;
         }
 
+        // Can't unwrap optionals if there is `ExpressibleByNilLiteral`
+        // conformance requirement placed on the type variable.
+        if (canBeNil)
+          break;
+
         // If this literal protocol is not a direct requirement it
-        // would be possible to change optionality while inferring
+        // would not be possible to change optionality while inferring
         // bindings for a supertype, so this hack doesn't apply.
         if (!isDirectRequirement)
           break;
@@ -472,31 +506,23 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
       binding.BindingType = type;
   }
 
-  // If this is not a literal protocol or it has been "covered" by an existing
-  // binding it can't provide a default type.
-  auto isUnviableForDefaulting = [&literalProtocols](ProtocolDecl *protocol) {
-    auto literal = literalProtocols.find(protocol);
-    return literal == literalProtocols.end() || literal->second.second;
-  };
-
   for (auto *constraint : Protocols) {
     auto *protocol = constraint->getProtocol();
 
-    if (isUnviableForDefaulting(protocol))
+    auto literal = literalProtocols.find(protocol);
+    if (literal == literalProtocols.end())
       continue;
 
-    // Let's try to coalesce integer and floating point literal protocols
-    // if they appear together because the only possible default type that
-    // could satisfy both requirements is `Double`.
-    if (protocol->isSpecificProtocol(
-          KnownProtocolKind::ExpressibleByIntegerLiteral)) {
-      auto *floatLiteral = cs.getASTContext().getProtocol(
-          KnownProtocolKind::ExpressibleByFloatLiteral);
-      // If `ExpressibleByFloatLiteral` is a requirement and it isn't
-      // covered, let's skip `ExpressibleByIntegerLiteral` requirement.
-      if (!isUnviableForDefaulting(floatLiteral))
-        continue;
-    }
+    bool isDirectRequirement = false;
+    bool isCovered = false;
+
+    std::tie(isDirectRequirement, isCovered) = literal->second;
+
+    // Can't be defaulted because it's already covered by an
+    // existing direct or transitive binding which is always
+    // better.
+    if (isCovered)
+      continue;
 
     auto defaultType = TypeChecker::getDefaultType(protocol, cs.DC);
     if (!defaultType)
@@ -509,9 +535,8 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
     // requirement or inferred transitive one to identify binding
     // kind correctly.
     addPotentialBinding({defaultType,
-                         isDirectRequirement(constraint)
-                             ? AllowedBindingKind::Subtypes
-                             : AllowedBindingKind::Supertypes,
+                         isDirectRequirement ? AllowedBindingKind::Subtypes
+                                             : AllowedBindingKind::Supertypes,
                          constraint});
   }
 }