Merge pull request #40559 from slavapestov/typealias-in-protocol-fixes

Don't resolve protocol typealiases as DependentMemberTypes in Structural mode

Author: slavapestov

lib/AST/GenericSignatureBuilder.cpp

@@ -3998,8 +3998,16 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
                                             ProtocolDecl *proto,
                                             const RequirementSource *source,
                                             bool onlySameTypeConstraints) {
-  auto protocolSubMap = SubstitutionMap::getProtocolSubstitutions(
-      proto, selfType.getDependentType(*this), ProtocolConformanceRef(proto));
+  auto selfTy = selfType.getDependentType(*this);
+
+  auto subst = [&](Requirement req) -> Optional<Requirement> {
+    return req.subst(
+        [&](SubstitutableType *t) -> Type {
+          assert(isa<GenericTypeParamType>(t));
+          return selfTy;
+        },
+        MakeAbstractConformanceForGenericType());
+  };
 
   auto result = ConstraintResult::Resolved;
 
@@ -4015,7 +4023,7 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
       if (onlySameTypeConstraints && req.getKind() != RequirementKind::SameType)
         continue;
 
-      auto substReq = req.subst(protocolSubMap);
+      auto substReq = subst(req);
       auto reqResult = substReq
                            ? addRequirement(*substReq, innerSource, nullptr)
                            : ConstraintResult::Conflicting;
@@ -4045,8 +4053,9 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
 
         auto innerSource = FloatingRequirementSource::viaProtocolRequirement(
             source, proto, reqRepr->getSeparatorLoc(), /*inferred=*/false);
-        addRequirement(req, reqRepr, innerSource,
-                       &protocolSubMap, nullptr);
+
+        if (auto substReq = subst(req))
+          addRequirement(*substReq, reqRepr, innerSource, nullptr);
         return false;
       });
 
@@ -4159,7 +4168,7 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
                     source, proto, SourceLoc(), /*inferred=*/true);
 
     auto rawReq = Requirement(RequirementKind::SameType, firstType, secondType);
-    if (auto req = rawReq.subst(protocolSubMap))
+    if (auto req = subst(rawReq))
       addRequirement(*req, inferredSameTypeSource, proto->getParentModule());
   };
 
@@ -4189,8 +4198,10 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
 
           auto innerSource = FloatingRequirementSource::viaProtocolRequirement(
               source, proto, reqRepr->getSeparatorLoc(), /*inferred=*/false);
-          addRequirement(req, reqRepr, innerSource, &protocolSubMap,
-                         /*inferForModule=*/nullptr);
+          if (auto substReq = subst(req)) {
+            addRequirement(*substReq, reqRepr, innerSource,
+                           /*inferForModule=*/nullptr);
+          }
           return false;
         });
 
@@ -5171,28 +5182,19 @@ ConstraintResult
 GenericSignatureBuilder::addRequirement(const Requirement &req,
                                         FloatingRequirementSource source,
                                         ModuleDecl *inferForModule) {
-  return addRequirement(req, nullptr, source, nullptr, inferForModule);
+  return addRequirement(req, nullptr, source, inferForModule);
 }
 
 ConstraintResult
 GenericSignatureBuilder::addRequirement(const Requirement &req,
                                         const RequirementRepr *reqRepr,
                                         FloatingRequirementSource source,
-                                        const SubstitutionMap *subMap,
                                         ModuleDecl *inferForModule) {
-  // Local substitution for types in the requirement.
-  auto subst = [&](Type t) {
-    if (subMap)
-      return t.subst(*subMap);
-
-    return t;
-  };
-
-  auto firstType = subst(req.getFirstType());
+  auto firstType = req.getFirstType();
   switch (req.getKind()) {
   case RequirementKind::Superclass:
   case RequirementKind::Conformance: {
-    auto secondType = subst(req.getSecondType());
+    auto secondType = req.getSecondType();
 
     if (inferForModule) {
       inferRequirements(*inferForModule, firstType,
@@ -5220,7 +5222,7 @@ GenericSignatureBuilder::addRequirement(const Requirement &req,
   }
 
   case RequirementKind::SameType: {
-    auto secondType = subst(req.getSecondType());
+    auto secondType = req.getSecondType();
 
     if (inferForModule) {
       inferRequirements(*inferForModule, firstType,
@@ -8567,8 +8569,8 @@ InferredGenericSignatureRequest::evaluate(
         }
       }
 
-      builder.addRequirement(req, reqRepr, source, nullptr,
-                              lookupDC->getParentModule());
+      builder.addRequirement(req, reqRepr, source,
+                             lookupDC->getParentModule());
       return false;
     };
 

lib/AST/GenericSignatureBuilder.h

@@ -571,7 +571,6 @@ class GenericSignatureBuilder {
   ConstraintResult addRequirement(const Requirement &req,
                                   const RequirementRepr *reqRepr,
                                   FloatingRequirementSource source,
-                                  const SubstitutionMap *subMap,
                                   ModuleDecl *inferForModule);
 
   /// Add all of a generic signature's parameters and requirements.

lib/Sema/TypeCheckType.cpp

@@ -220,50 +220,6 @@ Type TypeResolution::resolveDependentMemberType(
                                               baseTy);
 }
 
-Type TypeResolution::resolveSelfAssociatedType(Type baseTy,
-                                               DeclContext *DC,
-                                               Identifier name) const {
-  switch (stage) {
-  case TypeResolutionStage::Structural:
-    return DependentMemberType::get(baseTy, name);
-
-  case TypeResolutionStage::Interface:
-    // Handled below.
-    break;
-  }
-
-  assert(stage == TypeResolutionStage::Interface);
-  auto genericSig = getGenericSignature();
-  if (!genericSig)
-    return ErrorType::get(baseTy);
-
-  // Look for a nested type with the given name.
-  auto nestedType = genericSig->lookupNestedType(baseTy, name);
-  assert(nestedType);
-
-  // If the nested type has been resolved to an associated type, use it.
-  if (auto assocType = dyn_cast<AssociatedTypeDecl>(nestedType)) {
-    return DependentMemberType::get(baseTy, assocType);
-  }
-
-  if (nestedType->getDeclContext()->getSelfClassDecl()) {
-    // We found a member of a class from a protocol or protocol
-    // extension.
-    //
-    // Get the superclass of the 'Self' type parameter.
-    if (auto concreteTy = genericSig->getConcreteType(baseTy))
-      baseTy = concreteTy;
-    else {
-      baseTy = genericSig->getSuperclassBound(baseTy);
-      assert(baseTy);
-    }
-    assert(baseTy);
-  }
-
-  return TypeChecker::substMemberTypeWithBase(DC->getParentModule(), nestedType,
-                                              baseTy);
-}
-
 bool TypeResolution::areSameType(Type type1, Type type2) const {
   if (type1->isEqual(type2))
     return true;
@@ -450,23 +406,14 @@ Type TypeResolution::resolveTypeInContext(TypeDecl *typeDecl,
 
     if (selfType->is<GenericTypeParamType>()) {
       if (typeDecl->getDeclContext()->getSelfProtocolDecl()) {
-        if (isa<AssociatedTypeDecl>(typeDecl) ||
-            (isa<TypeAliasDecl>(typeDecl) &&
-             !cast<TypeAliasDecl>(typeDecl)->isGeneric())) {
-          // FIXME: We should use this lookup method for the Interface
-          // stage too, but right now that causes problems with
-          // Sequence.SubSequence vs Collection.SubSequence; the former
-          // is more canonical, but if we return that instead of the
-          // latter, we infer the wrong associated type in some cases,
-          // because we use the Sequence.SubSequence default instead of
-          // the Collection.SubSequence default, even when the conforming
-          // type wants to conform to Collection.
-          if (getStage() == TypeResolutionStage::Structural) {
-            return resolveSelfAssociatedType(selfType, foundDC,
-                                             typeDecl->getName());
-          } else if (auto assocType = dyn_cast<AssociatedTypeDecl>(typeDecl)) {
-            typeDecl = assocType->getAssociatedTypeAnchor();
-          }
+        auto *aliasDecl = dyn_cast<TypeAliasDecl>(typeDecl);
+        if (getStage() == TypeResolutionStage::Structural &&
+            aliasDecl && !aliasDecl->isGeneric()) {
+          return aliasDecl->getStructuralType();
+        }
+
+        if (auto assocType = dyn_cast<AssociatedTypeDecl>(typeDecl)) {
+          typeDecl = assocType->getAssociatedTypeAnchor();
         }
       }
 
@@ -543,10 +490,6 @@ bool TypeChecker::checkContextualRequirements(GenericTypeDecl *decl,
       noteLoc = loc;
   }
 
-  if (contextSig) {
-    parentTy = contextSig.getGenericEnvironment()->mapTypeIntoContext(parentTy);
-  }
-
   const auto subMap = parentTy->getContextSubstitutions(decl->getDeclContext());
   const auto genericSig = decl->getGenericSignature();
 
@@ -556,7 +499,16 @@ bool TypeChecker::checkContextualRequirements(GenericTypeDecl *decl,
         decl->getDeclaredInterfaceType(),
         genericSig.getGenericParams(),
         genericSig.getRequirements(),
-        QueryTypeSubstitutionMap{subMap});
+        [&](SubstitutableType *type) -> Type {
+          auto result = QueryTypeSubstitutionMap{subMap}(type);
+          if (result->hasTypeParameter()) {
+            if (contextSig) {
+              auto *genericEnv = contextSig.getGenericEnvironment();
+              return genericEnv->mapTypeIntoContext(result);
+            }
+          }
+          return result;
+        });
 
   switch (result) {
   case RequirementCheckResult::Failure:
@@ -845,15 +797,6 @@ Type TypeResolution::applyUnboundGenericArguments(
     // Check the generic arguments against the requirements of the declaration's
     // generic signature.
 
-    // First, map substitutions into context.
-    TypeSubstitutionMap contextualSubs = subs;
-    if (const auto contextSig = getGenericSignature()) {
-      auto *genericEnv = contextSig.getGenericEnvironment();
-      for (auto &pair : contextualSubs) {
-        pair.second = genericEnv->mapTypeIntoContext(pair.second);
-      }
-    }
-
     SourceLoc noteLoc = decl->getLoc();
     if (noteLoc.isInvalid())
       noteLoc = loc;
@@ -862,7 +805,16 @@ Type TypeResolution::applyUnboundGenericArguments(
         module, loc, noteLoc,
         UnboundGenericType::get(decl, parentTy, getASTContext()),
         genericSig.getGenericParams(), genericSig.getRequirements(),
-        QueryTypeSubstitutionMap{contextualSubs});
+        [&](SubstitutableType *type) -> Type {
+          auto result = QueryTypeSubstitutionMap{subs}(type);
+          if (result->hasTypeParameter()) {
+            if (const auto contextSig = getGenericSignature()) {
+              auto *genericEnv = contextSig.getGenericEnvironment();
+              return genericEnv->mapTypeIntoContext(result);
+            }
+          }
+          return result;
+        });
 
     switch (result) {
     case RequirementCheckResult::Failure:
@@ -2478,14 +2430,7 @@ TypeResolver::resolveAttributedType(TypeAttributes &attrs, TypeRepr *repr,
   // context, and then set isNoEscape if @escaping is not present.
   if (!ty) ty = resolveType(repr, instanceOptions);
   if (!ty || ty->hasError()) return ty;
-
-  // Type aliases inside protocols are not yet resolved in the structural
-  // stage of type resolution
-  if (ty->is<DependentMemberType>() &&
-      resolution.getStage() == TypeResolutionStage::Structural) {
-    return ty;
-  }
-
+  
   // Handle @escaping
   if (ty->is<FunctionType>()) {
     if (attrs.has(TAK_escaping)) {
@@ -3463,7 +3408,7 @@ NeverNullType TypeResolver::resolveArrayType(ArrayTypeRepr *repr,
     return ErrorType::get(getASTContext());
   }
 
-  ASTContext &ctx = baseTy->getASTContext();
+  ASTContext &ctx = getASTContext();
   // If the standard library isn't loaded, we ought to let the user know
   // something has gone terribly wrong, since the rest of the compiler is going
   // to assume it can canonicalize [T] to Array<T>.

lib/Sema/TypeCheckType.h

@@ -452,11 +452,6 @@ class TypeResolution {
                                   SourceRange baseRange,
                                   ComponentIdentTypeRepr *ref) const;
 
-  /// Resolve an unqualified reference to an associated type or type alias
-  /// in a protocol.
-  Type resolveSelfAssociatedType(Type baseTy, DeclContext *DC,
-                                 Identifier name) const;
-
   /// Determine whether the given two types are equivalent within this
   /// type resolution context.
   bool areSameType(Type type1, Type type2) const;

test/decl/protocol/req/associated_type_ambiguity.swift

@@ -1,36 +1,27 @@
 // RUN: %target-typecheck-verify-swift
 
-// Reference to associated type from 'where' clause should not be
+// References to associated type from 'where' clause should be
 // ambiguous when there's a typealias with the same name in another
-// protocol.
-//
-// FIXME: The semantics here are still really iffy. There's also a
-// case to be made that type aliases in protocol extensions should
-// only act as defaults and not as same-type constraints. However,
-// if we decide to go down that route, it's important that *both*
-// the unqualified (T) and qualified (Self.T) lookups behave the
-// same.
+// unrelated protocol.
+
 protocol P1 {
-  typealias T = Int // expected-note {{found this candidate}}
+  typealias T = Int // expected-note 3{{found this candidate}}
 }
 
 protocol P2 {
-  associatedtype T // expected-note {{found this candidate}}
+  associatedtype T // expected-note 3{{found this candidate}}
 }
 
-// FIXME: This extension's generic signature is still minimized differently from
-// the next one. We need to decide if 'T == Int' is a redundant requirement or
-// not.
-extension P1 where Self : P2, T == Int {
-  func takeT1(_: T) {}
-  func takeT2(_: Self.T) {}
+extension P1 where Self : P2, T == Int { // expected-error {{'T' is ambiguous for type lookup in this context}}
+  func takeT11(_: T) {} // expected-error {{'T' is ambiguous for type lookup in this context}}
+  func takeT12(_: Self.T) {}
 }
 
 extension P1 where Self : P2 {
   // FIXME: This doesn't make sense -- either both should
   // succeed, or both should be ambiguous.
-  func takeT1(_: T) {} // expected-error {{'T' is ambiguous for type lookup in this context}}
-  func takeT2(_: Self.T) {}
+  func takeT21(_: T) {} // expected-error {{'T' is ambiguous for type lookup in this context}}
+  func takeT22(_: Self.T) {}
 }
 
 // Same as above, but now we have two visible associated types with the same
@@ -39,20 +30,17 @@ protocol P3 {
   associatedtype T
 }
 
-// FIXME: This extension's generic signature is still minimized differently from
-// the next one. We need to decide if 'T == Int' is a redundant requirement or
-// not.
 extension P2 where Self : P3, T == Int {
-  func takeT1(_: T) {}
-  func takeT2(_: Self.T) {}
+  func takeT31(_: T) {}
+  func takeT32(_: Self.T) {}
 }
 
 extension P2 where Self : P3 {
-  func takeT1(_: T) {}
-  func takeT2(_: Self.T) {}
+  func takeT41(_: T) {}
+  func takeT42(_: Self.T) {}
 }
 
 protocol P4 : P2, P3 {
-  func takeT1(_: T)
-  func takeT2(_: Self.T)
+  func takeT51(_: T)
+  func takeT52(_: Self.T)
 }

test/decl/typealias/protocol.swift

@@ -268,7 +268,7 @@ extension P10 {
   typealias U = Float
 }
 
-extension P10 where T == Int { } // expected-warning{{neither type in same-type constraint ('Self.T' (aka 'Int') or 'Int') refers to a generic parameter or associated type}}
+extension P10 where T == Int { } // expected-warning{{neither type in same-type constraint ('Int' or 'Int') refers to a generic parameter or associated type}}
 
 extension P10 where A == X<T> { }
 
@@ -277,7 +277,7 @@ extension P10 where A == X<U> { }
 extension P10 where A == X<Self.U> { }
 
 extension P10 where V == Int { } // expected-warning {{'V' is deprecated: just use Int, silly}}
-// expected-warning@-1{{neither type in same-type constraint ('Self.V' (aka 'Int') or 'Int') refers to a generic parameter or associated type}}
+// expected-warning@-1{{neither type in same-type constraint ('Int' or 'Int') refers to a generic parameter or associated type}}
 
 // rdar://problem/36003312
 protocol P11 {