Sema: Require CS to certify conversions associated w/ existential member accesses

Author: AnthonyLatsis

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -287,6 +287,11 @@ SIMPLE_LOCATOR_PATH_ELT(CoercionOperand)
 /// A fallback type for some AST location (i.e. key path literal).
 SIMPLE_LOCATOR_PATH_ELT(FallbackType)
 
+/// An implicit conversion (upcast) associated with an existential member access
+/// performed to abstract the member reference type away from context-specific
+/// types like `Self` that are not well-formed in the access context.
+SIMPLE_LOCATOR_PATH_ELT(ExistentialMemberAccessConversion)
+
 #undef LOCATOR_PATH_ELT
 #undef CUSTOM_LOCATOR_PATH_ELT
 #undef SIMPLE_LOCATOR_PATH_ELT

include/swift/Sema/ConstraintSystem.h

@@ -4442,8 +4442,7 @@ class ConstraintSystem {
   /// determine the reference type of the member reference.
   Type getMemberReferenceTypeFromOpenedType(
       Type &openedType, Type baseObjTy, ValueDecl *value, DeclContext *outerDC,
-      ConstraintLocator *locator, bool hasAppliedSelf,
-      bool isStaticMemberRefOnProtocol, bool isDynamicResult,
+      ConstraintLocator *locator, bool hasAppliedSelf, bool isDynamicLookup,
       OpenedTypeMap &replacements);
 
   /// Retrieve the type of a reference to the given value declaration,
@@ -4453,16 +4452,14 @@ class ConstraintSystem {
   /// this routine "opens up" the type by replacing each instance of a generic
   /// parameter with a fresh type variable.
   ///
-  /// \param isDynamicResult Indicates that this declaration was found via
+  /// \param isDynamicLookup Indicates that this declaration was found via
   /// dynamic lookup.
   ///
   /// \returns a description of the type of this declaration reference.
   DeclReferenceType getTypeOfMemberReference(
-                          Type baseTy, ValueDecl *decl, DeclContext *useDC,
-                          bool isDynamicResult,
-                          FunctionRefKind functionRefKind,
-                          ConstraintLocator *locator,
-                          OpenedTypeMap *replacements = nullptr);
+      Type baseTy, ValueDecl *decl, DeclContext *useDC, bool isDynamicLookup,
+      FunctionRefKind functionRefKind, ConstraintLocator *locator,
+      OpenedTypeMap *replacements = nullptr);
 
   /// Retrieve a list of generic parameter types solver has "opened" (replaced
   /// with a type variable) at the given location.

lib/Sema/CSDiagnostics.h

@@ -1323,17 +1323,18 @@ class UnintendedExtraGenericParamMemberFailure final
   bool diagnoseAsError() override;
 };
 
-/// Diagnose cases where a member only accessible on generic constraints
-/// requiring conformance to a protocol is used on a value of the
-/// existential protocol type e.g.
+/// Diagnose cases where a protocol member cannot be accessed with an
+/// existential, e.g. due to occurrences of `Self` in non-covariant position in
+/// the type of the member reference:
 ///
 /// ```swift
+/// struct G<T> {}
 /// protocol P {
-///   var foo: Self { get }
+///   func foo() -> G<Self>
 /// }
 ///
-/// func bar<X : P>(p: X) {
-///   p.foo
+/// func bar(p: any P) {
+///   p.foo()
 /// }
 /// ```
 class InvalidMemberRefOnExistential final : public InvalidMemberRefFailure {

lib/Sema/CSSimplify.cpp

@@ -5620,6 +5620,23 @@ bool ConstraintSystem::repairFailures(
     }
   }
 
+  // If there is a conversion associated with an existential member access
+  // along the path, the problem is that the constraint system does not support
+  // the (formally sane) upcast required to access the member.
+  if (llvm::find_if(path, [](const LocatorPathElt &elt) -> bool {
+        return elt.is<LocatorPathElt::ExistentialMemberAccessConversion>();
+      }) != path.end()) {
+    if (auto overload = findSelectedOverloadFor(castToExpr(anchor))) {
+      auto &choice = overload->choice;
+      conversionsOrFixes.push_back(AllowMemberRefOnExistential::create(
+          *this, choice.getBaseType(), choice.getDecl(),
+          DeclNameRef(choice.getDecl()->getName()),
+          getConstraintLocator(locator)));
+
+      return true;
+    }
+  }
+
   auto elt = path.back();
   switch (elt.getKind()) {
   case ConstraintLocator::LValueConversion: {
@@ -15153,7 +15170,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::GenericArgumentsMismatch:
   case FixKind::AllowConcreteTypeSpecialization:
   case FixKind::IgnoreGenericSpecializationArityMismatch:
-  case FixKind::IgnoreKeyPathSubscriptIndexMismatch: {
+  case FixKind::IgnoreKeyPathSubscriptIndexMismatch:
+  case FixKind::AllowMemberRefOnExistential: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
   case FixKind::IgnoreThrownErrorMismatch: {
@@ -15376,7 +15394,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::RemoveCall:
   case FixKind::RemoveUnwrap:
   case FixKind::DefineMemberBasedOnUse:
-  case FixKind::AllowMemberRefOnExistential:
   case FixKind::AllowTypeOrInstanceMember:
   case FixKind::AllowInvalidPartialApplication:
   case FixKind::AllowInvalidInitRef:

lib/Sema/ConstraintLocator.cpp

@@ -112,6 +112,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::ThrownErrorType:
   case ConstraintLocator::FallbackType:
   case ConstraintLocator::KeyPathSubscriptIndex:
+  case ConstraintLocator::ExistentialMemberAccessConversion:
     return 0;
 
   case ConstraintLocator::FunctionArgument:
@@ -532,6 +533,9 @@ void LocatorPathElt::dump(raw_ostream &out) const {
     out << "key path subscript index parameter";
     break;
   }
+  case ConstraintLocator::ExistentialMemberAccessConversion:
+    out << "existential member access conversion";
+    break;
   }
 }
 

lib/Sema/ConstraintSystem.cpp

@@ -1316,6 +1316,14 @@ Type ConstraintSystem::getFixedTypeRecursive(Type type, TypeMatchOptions &flags,
     return getFixedTypeRecursive(simplified, flags, wantRValue);
   }
 
+  if (auto metatype = type->getAs<AnyMetatypeType>()) {
+    auto simplified = simplifyType(type);
+    if (simplified.getPointer() == type.getPointer())
+      return type;
+
+    return getFixedTypeRecursive(simplified, flags, wantRValue);
+  }
+
   if (auto typeVar = type->getAs<TypeVariableType>()) {
     if (auto fixed = getFixedType(typeVar))
       return getFixedTypeRecursive(fixed, flags, wantRValue);
@@ -2420,10 +2428,25 @@ Type constraints::typeEraseOpenedArchetypesWithRoot(
       /*force=*/true);
 }
 
+static bool isExistentialMemberAccessWithExplicitBaseExpression(
+    Type baseInstanceTy, ValueDecl *member, ConstraintLocator *locator,
+    bool isDynamicLookup) {
+  if (isDynamicLookup) {
+    return false;
+  }
+
+  // '.x' does not have an explicit base expression.
+  if (locator->isLastElement<LocatorPathElt::UnresolvedMember>()) {
+    return false;
+  }
+
+  return baseInstanceTy->isExistentialType() &&
+         member->getDeclContext()->getSelfProtocolDecl();
+}
+
 Type ConstraintSystem::getMemberReferenceTypeFromOpenedType(
     Type &openedType, Type baseObjTy, ValueDecl *value, DeclContext *outerDC,
-    ConstraintLocator *locator, bool hasAppliedSelf,
-    bool isStaticMemberRefOnProtocol, bool isDynamicResult,
+    ConstraintLocator *locator, bool hasAppliedSelf, bool isDynamicLookup,
     OpenedTypeMap &replacements) {
   Type type = openedType;
 
@@ -2450,7 +2473,7 @@ Type ConstraintSystem::getMemberReferenceTypeFromOpenedType(
 
   // Check if we need to apply a layer of optionality to the uncurried type.
   if (!isRequirementOrWitness(locator)) {
-    if (isDynamicResult || value->getAttrs().hasAttribute<OptionalAttr>()) {
+    if (isDynamicLookup || value->getAttrs().hasAttribute<OptionalAttr>()) {
       const auto applyOptionality = [&](FunctionType *fnTy) -> Type {
         Type resultTy;
         // Optional and dynamic subscripts are a special case, because the
@@ -2490,12 +2513,13 @@ Type ConstraintSystem::getMemberReferenceTypeFromOpenedType(
     type = type->replaceSelfParameterType(baseObjTy);
   }
 
-  // Superficially, protocol members with an existential base are accessed
-  // directly on the existential, and not an opened archetype, and we may have
-  // to adjust the type of the reference (e.g. covariant 'Self' type-erasure) to
-  // support certain accesses.
-  if (!isStaticMemberRefOnProtocol && !isDynamicResult &&
-      baseObjTy->isExistentialType() && outerDC->getSelfProtocolDecl() &&
+  // From the user perspective, protocol members that are accessed with an
+  // existential base are accessed directly on the existential, and not an
+  // opened archetype, so the type of the member reference must be abstracted
+  // away (upcast) from context-specific types like `Self` in covariant
+  // position.
+  if (isExistentialMemberAccessWithExplicitBaseExpression(
+          baseObjTy, value, locator, isDynamicLookup) &&
       // If there are no type variables, there were no references to 'Self'.
       type->hasTypeVariable()) {
     const auto selfGP = cast<GenericTypeParamType>(
@@ -2504,13 +2528,6 @@ Type ConstraintSystem::getMemberReferenceTypeFromOpenedType(
 
     type = typeEraseOpenedExistentialReference(type, baseObjTy, openedTypeVar,
                                                TypePosition::Covariant);
-
-    Type contextualTy;
-
-    if (auto *anchor = getAsExpr(simplifyLocatorToAnchor(locator))) {
-      contextualTy =
-          getContextualType(getParentExpr(anchor), /*forConstraint=*/false);
-    }
   }
 
   // Construct an idealized parameter type of the initializer associated
@@ -2591,12 +2608,9 @@ bool ConstraintSystem::isPartialApplication(ConstraintLocator *locator) {
   return level < (baseTy->is<MetatypeType>() ? 1 : 2);
 }
 
-DeclReferenceType
-ConstraintSystem::getTypeOfMemberReference(
-    Type baseTy, ValueDecl *value, DeclContext *useDC,
-    bool isDynamicResult,
-    FunctionRefKind functionRefKind,
-    ConstraintLocator *locator,
+DeclReferenceType ConstraintSystem::getTypeOfMemberReference(
+    Type baseTy, ValueDecl *value, DeclContext *useDC, bool isDynamicLookup,
+    FunctionRefKind functionRefKind, ConstraintLocator *locator,
     OpenedTypeMap *replacementsPtr) {
   // Figure out the instance type used for the base.
   Type resolvedBaseTy = getFixedTypeRecursive(baseTy, /*wantRValue=*/true);
@@ -2619,10 +2633,11 @@ ConstraintSystem::getTypeOfMemberReference(
   // metatype and `bar` is static member declared in a protocol  or its
   // extension.
   bool isStaticMemberRefOnProtocol = false;
-  if (resolvedBaseTy->is<MetatypeType>() && baseObjTy->isExistentialType() &&
-      value->isStatic()) {
-    isStaticMemberRefOnProtocol =
-        locator->isLastElement<LocatorPathElt::UnresolvedMember>();
+  if (baseObjTy->isExistentialType() && value->isStatic() &&
+      locator->isLastElement<LocatorPathElt::UnresolvedMember>()) {
+    assert(resolvedBaseTy->is<MetatypeType>() &&
+           "Assumed base of unresolved member access must be a metatype");
+    isStaticMemberRefOnProtocol = true;
   }
 
   if (auto *typeDecl = dyn_cast<TypeDecl>(value)) {
@@ -2791,7 +2806,7 @@ ConstraintSystem::getTypeOfMemberReference(
     // if it didn't conform.
     addConstraint(ConstraintKind::Bind, baseOpenedTy, selfObjTy,
                   getConstraintLocator(locator));
-  } else if (!isDynamicResult) {
+  } else if (!isDynamicLookup) {
     addSelfConstraint(*this, baseOpenedTy, selfObjTy, locator);
   }
 
@@ -2857,14 +2872,14 @@ ConstraintSystem::getTypeOfMemberReference(
   // Compute the type of the reference.
   Type type = getMemberReferenceTypeFromOpenedType(
       openedType, baseObjTy, value, outerDC, locator, hasAppliedSelf,
-      isStaticMemberRefOnProtocol, isDynamicResult, replacements);
+      isDynamicLookup, replacements);
 
   // Do the same thing for the original type, if there can be any difference.
   Type origType = type;
   if (openedType.getPointer() != origOpenedType.getPointer()) {
     origType = getMemberReferenceTypeFromOpenedType(
         origOpenedType, baseObjTy, value, outerDC, locator, hasAppliedSelf,
-        isStaticMemberRefOnProtocol, isDynamicResult, replacements);
+        isDynamicLookup, replacements);
   }
 
   // If we opened up any type variables, record the replacements.
@@ -4035,6 +4050,35 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
   }
 
   if (auto *decl = choice.getDeclOrNull()) {
+    // If this is an existential member access and adjustments were made to the
+    // member reference type, require that the constraint system is happy with
+    // the ensuing conversion.
+    if (auto baseTy = choice.getBaseType()) {
+      baseTy = getFixedTypeRecursive(baseTy, /*wantRValue=*/true);
+      const auto instanceTy = baseTy->getMetatypeInstanceType();
+
+      if (isExistentialMemberAccessWithExplicitBaseExpression(
+              instanceTy, decl, locator,
+              /*isDynamicLookup=*/choice.getKind() ==
+                  OverloadChoiceKind::DeclViaDynamic)) {
+
+        // Strip curried 'self' parameters.
+        auto fromTy = openedType->castTo<AnyFunctionType>()->getResult();
+        auto toTy = refType;
+        if (!doesMemberRefApplyCurriedSelf(baseTy, decl)) {
+          toTy = toTy->castTo<AnyFunctionType>()->getResult();
+        }
+
+        if (!fromTy->isEqual(toTy)) {
+          ConstraintLocatorBuilder conversionLocator = locator;
+          conversionLocator = conversionLocator.withPathElement(
+              ConstraintLocator::ExistentialMemberAccessConversion);
+          addConstraint(ConstraintKind::Conversion, fromTy, toTy,
+                        conversionLocator);
+        }
+      }
+    }
+
     // If the declaration is unavailable, note that in the score.
     if (isDeclUnavailable(decl, locator))
       increaseScore(SK_Unavailable, locator);
@@ -6293,6 +6337,7 @@ void constraints::simplifyLocator(ASTNode &anchor,
     case ConstraintLocator::ImplicitlyUnwrappedDisjunctionChoice:
     case ConstraintLocator::FallbackType:
     case ConstraintLocator::KeyPathSubscriptIndex:
+    case ConstraintLocator::ExistentialMemberAccessConversion:
       break;
     }