[CS] Don't adjust opened types of @dynamicMemberLookup overloa… (#28694)

[CS] Don't adjust opened types of @dynamicMemberLookup overloads

Author: hamishknight

lib/Sema/CSApply.cpp

@@ -1376,28 +1376,17 @@ namespace {
       auto subscriptRef = resolveConcreteDeclRef(subscript, memberLoc);
 
       // Figure out the index and result types.
-      Type resultTy;
-      if (choice.getKind() != OverloadChoiceKind::DynamicMemberLookup &&
-          choice.getKind() != OverloadChoiceKind::KeyPathDynamicMemberLookup) {
-        auto subscriptTy = simplifyType(selected.openedType);
-        auto *subscriptFnTy = subscriptTy->castTo<FunctionType>();
-        resultTy = subscriptFnTy->getResult();
-
-        // Coerce the index argument.
-        index = coerceCallArguments(index, subscriptFnTy, subscriptRef, nullptr,
-                                    argLabels, hasTrailingClosure,
-                                    locator.withPathElement(
-                                      ConstraintLocator::ApplyArgument));
-        if (!index)
-          return nullptr;
+      auto subscriptTy = simplifyType(selected.openedType);
+      auto *subscriptFnTy = subscriptTy->castTo<FunctionType>();
+      auto resultTy = subscriptFnTy->getResult();
 
-      } else {
-        // If this is a @dynamicMemberLookup, then the type of the selection is
-        // actually the property/result type.  That's fine though, and we
-        // already have the index type adjusted to the correct type expected by
-        // the subscript.
-        resultTy = simplifyType(selected.openedType);
-      }
+      // Coerce the index argument.
+      index = coerceCallArguments(index, subscriptFnTy, subscriptRef, nullptr,
+                                  argLabels, hasTrailingClosure,
+                                  locator.withPathElement(
+                                    ConstraintLocator::ApplyArgument));
+      if (!index)
+        return nullptr;
 
       auto getType = [&](const Expr *E) -> Type {
         return cs.getType(E);
@@ -4490,9 +4479,6 @@ namespace {
               OverloadChoiceKind::KeyPathDynamicMemberLookup;
 
       if (forDynamicLookup) {
-        overload.openedType =
-            overload.openedFullType->castTo<AnyFunctionType>()->getResult();
-
         labels = cs.getASTContext().Id_dynamicMember;
 
         auto indexType = getTypeOfDynamicMemberIndex(overload);

lib/Sema/ConstraintSystem.cpp

@@ -1875,9 +1875,7 @@ isInvalidPartialApplication(ConstraintSystem &cs, const ValueDecl *member,
 
 std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
     const OverloadChoice &choice, ConstraintLocator *locator,
-    Type boundType, Type refType, DeclContext *useDC,
-    llvm::function_ref<void(unsigned int, Type, ConstraintLocator *)>
-        verifyThatArgumentIsHashable) {
+    Type boundType, Type refType) {
   // If the declaration is unavailable, note that in the score.
   if (choice.getDecl()->getAttrs().isUnavailable(getASTContext())) {
     increaseScore(SK_Unavailable);
@@ -1899,6 +1897,7 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
 
     // Deal with values declared as implicitly unwrapped, or
     // functions with return types that are implicitly unwrapped.
+    // TODO: Move this logic to bindOverloadType.
     if (choice.isImplicitlyUnwrappedValueOrReturnValue()) {
       // Build the disjunction to attempt binding both T? and T (or
       // function returning T? and function returning T).
@@ -1910,6 +1909,7 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
       bindConstraintCreated = true;
     }
 
+    // TODO: Move this to getTypeOfMemberReference.
     refType = OptionalType::get(refType->getRValueType());
   }
 
@@ -1922,6 +1922,7 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
   case OverloadChoiceKind::KeyPathApplication:
     return {refType, bindConstraintCreated};
   case OverloadChoiceKind::DeclViaDynamic: {
+    // TODO: Move the IUO handling logic here to bindOverloadType.
     if (isa<SubscriptDecl>(choice.getDecl())) {
       // We always expect function type for subscripts.
       auto fnTy = refType->castTo<AnyFunctionType>();
@@ -1982,20 +1983,52 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
 
       // We store an Optional of the originally resolved type in the
       // overload set.
+      // TODO: Move this to getTypeOfMemberReference.
       refType = OptionalType::get(refType->getRValueType());
     }
 
     return {refType, /*bindConstraintCreated*/ true};
   }
+  case OverloadChoiceKind::DynamicMemberLookup:
+  case OverloadChoiceKind::KeyPathDynamicMemberLookup:
+    return {refType, bindConstraintCreated};
+  }
+
+  llvm_unreachable("Unhandled OverloadChoiceKind in switch.");
+}
+
+void ConstraintSystem::bindOverloadType(
+    const SelectedOverload &overload, Type boundType,
+    ConstraintLocator *locator, DeclContext *useDC,
+    llvm::function_ref<void(unsigned int, Type, ConstraintLocator *)>
+        verifyThatArgumentIsHashable) {
+  auto choice = overload.choice;
+  auto openedType = overload.openedType;
+
+  auto bindTypeOrIUO = [&](Type ty) {
+    if (choice.isImplicitlyUnwrappedValueOrReturnValue()) {
+      // Build the disjunction to attempt binding both T? and T (or
+      // function returning T? and function returning T).
+      buildDisjunctionForImplicitlyUnwrappedOptional(boundType, ty, locator);
+    } else {
+      // Add the type binding constraint.
+      addConstraint(ConstraintKind::Bind, boundType, ty, locator);
+    }
+  };
+  switch (choice.getKind()) {
+  case OverloadChoiceKind::Decl:
+  case OverloadChoiceKind::DeclViaBridge:
+  case OverloadChoiceKind::DeclViaUnwrappedOptional:
+  case OverloadChoiceKind::TupleIndex:
+  case OverloadChoiceKind::BaseType:
+  case OverloadChoiceKind::KeyPathApplication:
+  case OverloadChoiceKind::DeclViaDynamic:
+    bindTypeOrIUO(openedType);
+    return;
   case OverloadChoiceKind::DynamicMemberLookup: {
     // DynamicMemberLookup results are always a (dynamicMember:T1)->T2
     // subscript.
-    auto refFnType = refType->castTo<FunctionType>();
-
-    // If this is a dynamic member lookup, then the decl we have is for the
-    // subscript(dynamicMember:) member, but the type we need to return is the
-    // result of the subscript.  Dig through it.
-    refType = refFnType->getResult();
+    auto refFnType = openedType->castTo<FunctionType>();
 
     // Before we drop the argument type on the floor, we need to constrain it
     // to having a literal conformance to ExpressibleByStringLiteral.  This
@@ -2008,7 +2041,7 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
         TypeChecker::getProtocol(getASTContext(), choice.getDecl()->getLoc(),
                                  KnownProtocolKind::ExpressibleByStringLiteral);
     if (!stringLiteral)
-      return {refType, bindConstraintCreated};
+      return;
 
     addConstraint(ConstraintKind::LiteralConformsTo, argType,
                   stringLiteral->getDeclaredType(), locator);
@@ -2018,19 +2051,21 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
     if (isa<KeyPathExpr>(locator->getAnchor()))
       verifyThatArgumentIsHashable(0, argType, locator);
 
-    return {refType, bindConstraintCreated};
+    // The resolved decl is for subscript(dynamicMember:), however the original
+    // member constraint was for a property. Therefore we need to bind to the
+    // result type.
+    bindTypeOrIUO(refFnType->getResult());
+    return;
   }
   case OverloadChoiceKind::KeyPathDynamicMemberLookup: {
-    auto *fnType = refType->castTo<FunctionType>();
+    auto *fnType = openedType->castTo<FunctionType>();
     assert(fnType->getParams().size() == 1 &&
            "subscript always has one argument");
     // Parameter type is KeyPath<T, U> where `T` is a root type
     // and U is a leaf type (aka member type).
     auto keyPathTy =
         fnType->getParams()[0].getPlainType()->castTo<BoundGenericType>();
 
-    refType = fnType->getResult();
-
     auto *keyPathDecl = keyPathTy->getAnyNominal();
     assert(isKnownKeyPathDecl(getASTContext(), keyPathDecl) &&
            "parameter is supposed to be a keypath");
@@ -2051,18 +2086,11 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
     DeclName memberName =
         isSubscriptRef ? DeclBaseName::createSubscript() : choice.getName();
 
-    auto *memberRef = Constraint::createMember(
-        *this, ConstraintKind::ValueMember, LValueType::get(rootTy), memberTy,
-        memberName, useDC,
-        isSubscriptRef ? FunctionRefKind::DoubleApply
-                       : FunctionRefKind::Unapplied,
-        keyPathLoc);
-
-    // Delay simplication of this constraint until after the overload choice
-    // has been bound for this key path dynamic member. This helps to identify
-    // recursive calls with the same base.
-    addUnsolvedConstraint(memberRef);
-    activateConstraint(memberRef);
+    addValueMemberConstraint(LValueType::get(rootTy), memberName, memberTy,
+                             useDC,
+                             isSubscriptRef ? FunctionRefKind::DoubleApply
+                                            : FunctionRefKind::Unapplied,
+                             /*outerAlternatives=*/{}, keyPathLoc);
 
     // In case of subscript things are more compicated comparing to "dot"
     // syntax, because we have to get "applicable function" constraint
@@ -2140,10 +2168,15 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
 
     if (isa<KeyPathExpr>(locator->getAnchor()))
       verifyThatArgumentIsHashable(0, keyPathTy, locator);
+
+    // The resolved decl is for subscript(dynamicMember:), however the
+    // original member constraint was either for a property, or we've
+    // re-purposed the overload type variable to represent the result type of
+    // the subscript. In both cases, we need to bind to the result type.
+    bindTypeOrIUO(fnType->getResult());
+    return;
   }
-    return {refType, bindConstraintCreated};
   }
-
   llvm_unreachable("Unhandled OverloadChoiceKind in switch.");
 }
 
@@ -2231,8 +2264,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
     // getTypeOfMemberReference(); their result types are unchecked
     // optional.
     std::tie(refType, bindConstraintCreated) =
-        adjustTypeOfOverloadReference(choice, locator, boundType, refType,
-                                      useDC, verifyThatArgumentIsHashable);
+        adjustTypeOfOverloadReference(choice, locator, boundType, refType);
     break;
   }
 
@@ -2361,15 +2393,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
 
   // In some cases we already created the appropriate bind constraints.
   if (!bindConstraintCreated) {
-    if (choice.isImplicitlyUnwrappedValueOrReturnValue()) {
-      // Build the disjunction to attempt binding both T? and T (or
-      // function returning T? and function returning T).
-      buildDisjunctionForImplicitlyUnwrappedOptional(boundType, refType,
-                                                     locator);
-    } else {
-      // Add the type binding constraint.
-      addConstraint(ConstraintKind::Bind, boundType, refType, locator);
-    }
+    bindOverloadType(overload, boundType, locator, useDC,
+                     verifyThatArgumentIsHashable);
   }
 
   if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {

lib/Sema/ConstraintSystem.h

@@ -2817,7 +2817,12 @@ class ConstraintSystem {
   /// this member and a bit indicating whether or not a bind constraint was added.
   std::pair<Type, bool> adjustTypeOfOverloadReference(
       const OverloadChoice &choice, ConstraintLocator *locator, Type boundType,
-      Type refType, DeclContext *useDC,
+      Type refType);
+
+  /// Add the constraints needed to bind an overload's type variable.
+  void bindOverloadType(
+      const SelectedOverload &overload, Type boundType,
+      ConstraintLocator *locator, DeclContext *useDC,
       llvm::function_ref<void(unsigned int, Type, ConstraintLocator *)>
           verifyThatArgumentIsHashable);