Merge pull request #18188 from gregomni/disambiguateOptionalPatternBinding

[Sema] Constrain result type of expressions in optional pattern bindings

Author: gregomni

lib/Sema/CSDiag.cpp

@@ -313,6 +313,11 @@ void constraints::simplifyLocator(Expr *&anchor,
         }
       }
       break;
+
+    case ConstraintLocator::ContextualType:
+      // This was just for identifying purposes, strip it off.
+      path = path.slice(1);
+      continue;
 
     default:
       // FIXME: Lots of other cases to handle.

lib/Sema/CSSolver.cpp

@@ -1417,16 +1417,19 @@ ConstraintSystem::solveImpl(Expr *&expr,
     if (getContextualTypePurpose() == CTP_YieldByReference)
       constraintKind = ConstraintKind::Bind;
 
+    auto *convertTypeLocator = getConstraintLocator(
+        getConstraintLocator(expr), ConstraintLocator::ContextualType);
+
     if (allowFreeTypeVariables == FreeTypeVariableBinding::UnresolvedType) {
       convertType = convertType.transform([&](Type type) -> Type {
         if (type->is<UnresolvedType>())
-          return createTypeVariable(getConstraintLocator(expr));
+          return createTypeVariable(convertTypeLocator);
         return type;
       });
     }
 
     addConstraint(constraintKind, getType(expr), convertType,
-                  getConstraintLocator(expr), /*isFavored*/ true);
+                  convertTypeLocator, /*isFavored*/ true);
   }
 
   // Notify the listener that we've built the constraint system.

lib/Sema/ConstraintLocator.cpp

@@ -80,6 +80,7 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
     case TypeParameterRequirement:
     case ImplicitlyUnwrappedDisjunctionChoice:
     case DynamicLookupResult:
+    case ContextualType:
       if (unsigned numValues = numNumericValuesInPathElement(elt.getKind())) {
         id.AddInteger(elt.getValue());
         if (numValues > 1)
@@ -258,6 +259,10 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
     case DynamicLookupResult:
       out << "dynamic lookup result";
       break;
+
+    case ContextualType:
+      out << "contextual type";
+      break;
     }
   }
 

lib/Sema/ConstraintLocator.h

@@ -131,8 +131,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     TypeParameterRequirement,
     /// \brief Locator for a binding from an IUO disjunction choice.
     ImplicitlyUnwrappedDisjunctionChoice,
-    /// \brief A result of an expressoin involving dynamic lookup.
+    /// \brief A result of an expression involving dynamic lookup.
     DynamicLookupResult,
+    /// \brief The desired contextual type passed in to the constraint system.
+    ContextualType,
   };
 
   /// \brief Determine the number of numeric values used for the given path
@@ -167,6 +169,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     case OpenedGeneric:
     case ImplicitlyUnwrappedDisjunctionChoice:
     case DynamicLookupResult:
+    case ContextualType:
       return 0;
 
     case GenericArgument:
@@ -231,6 +234,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     case TypeParameterRequirement:
     case ImplicitlyUnwrappedDisjunctionChoice:
     case DynamicLookupResult:
+    case ContextualType:
       return 0;
 
     case FunctionArgument:

lib/Sema/TypeCheckConstraints.cpp

@@ -2040,9 +2040,18 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
   if (options.contains(TypeCheckExprFlags::AllowUnresolvedTypeVariables))
     allowFreeTypeVariables = FreeTypeVariableBinding::UnresolvedType;
 
+  Type convertTo = convertType.getType();
+  if (options.contains(TypeCheckExprFlags::ExpressionTypeMustBeOptional)) {
+    assert(!convertTo && "convertType and type check options conflict");
+    auto *convertTypeLocator = cs.getConstraintLocator(
+        cs.getConstraintLocator(expr), ConstraintLocator::ContextualType);
+    Type var = cs.createTypeVariable(convertTypeLocator);
+    convertTo = getOptionalType(expr->getLoc(), var);
+  }
+
   // Attempt to solve the constraint system.
   SmallVector<Solution, 4> viable;
-  if (cs.solve(expr, convertType.getType(), listener, viable,
+  if (cs.solve(expr, convertTo, listener, viable,
                allowFreeTypeVariables))
     return Type();
 
@@ -2439,6 +2448,8 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
 
   TypeLoc contextualType;
   auto contextualPurpose = CTP_Unused;
+  TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
+
   if (pattern->hasType()) {
     contextualType = TypeLoc::withoutLoc(pattern->getType());
     contextualPurpose = CTP_Initialization;
@@ -2453,10 +2464,11 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
       if (isa<NamedPattern>(inner) || isa<AnyPattern>(inner))
         contextualType = typedPattern->getTypeLoc();
     }
+  } else if (isa<OptionalSomePattern>(pattern)) {
+    flags |= TypeCheckExprFlags::ExpressionTypeMustBeOptional;
   }
 
   // Type-check the initializer.
-  TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
   if (skipApplyingSolution)
     flags |= TypeCheckExprFlags::SkipApplyingSolution;
 

lib/Sema/TypeChecker.h

@@ -261,6 +261,10 @@ enum class TypeCheckExprFlags {
 
   /// This is an inout yield.
   IsInOutYield = 0x200,
+
+  /// If set, a conversion constraint should be specfied so that the result of
+  /// the expression is an optional type.
+  ExpressionTypeMustBeOptional = 0x400,
 };
 
 using TypeCheckExprOptions = OptionSet<TypeCheckExprFlags>;

test/Constraints/patterns.swift

@@ -351,3 +351,57 @@ func testOne() {
     if case One.E.SomeError = error {} // expected-error{{generic enum type 'One.E' is ambiguous without explicit generic parameters when matching value of type 'Error'}}
   }
 }
+
+// SR-8347
+// constrain initializer expressions of optional some pattern bindings to be optional
+func test8347() -> String {
+  struct C {
+    subscript (s: String) -> String? {
+      return ""
+    }
+    subscript (s: String) -> [String] {
+      return [""]
+    }
+
+    func f() -> String? {
+      return ""
+    }
+    func f() -> Int {
+      return 3
+    }
+
+    func g() -> String {
+      return ""
+    }
+
+    func h() -> String {
+      return ""
+    }
+    func h() -> Double {
+      return 3.0
+    }
+    func h() -> Int? { //expected-note{{found this candidate}}
+      return 2
+    }
+    func h() -> Float? { //expected-note{{found this candidate}}
+      return nil
+    }
+
+  }
+
+  let c = C()
+  if let s = c[""] {
+    return s
+  }
+  if let s = c.f() {
+    return s
+  }
+  if let s = c.g() { //expected-error{{initializer for conditional binding must have Optional type, not 'String'}}
+    return s
+  }
+  if let s = c.h() { //expected-error{{ambiguous use of 'h()'}}
+    return s
+  }
+}
+
+