GSB: Fix maybeResolveEquivalenceClass() with member type of superclass-constrained type

Name lookup might find an associated type whose protocol is not in our
conforms-to list, if we have a superclass constraint and the superclass
conforms to the associated type's protocol.

We used to return an unresolved type in this case, which would result in
the constraint getting delayed forever and dropped.

While playing wack-a-mole with regressing crashers, I had to do some
refactoring to get all the tests to pass. Unfortuanately these refactorings
don't lend themselves well to being peeled off into their own commits:

- maybeAddSameTypeRequirementForNestedType() was almost identical to
  concretizeNestedTypeFromConcreteParent(), except for superclasses
  instead of concrete same-type constraints. I merged them together.

- We used to drop same-type constraints where the subject type was an
  ErrorType, because maybeResolveEquivalenceClass() would return an
  unresolved type in this case.

  This violated some invariants around nested types of ArchetypeTypes,
  because now it was possible for a nested type of a concrete type to
  be non-concrete, if the type witness in the conformance was missing
  due to an error.

  Fix this by removing the ErrorType hack, and adjusting a couple of
  other places to handle ErrorTypes in order to avoid regressing with
  invalid code.

Fixes <rdar://problem/45216921>, <https://bugs.swift.org/browse/SR-8945>,
<https://bugs.swift.org/browse/SR-12744>.

Author: slavapestov

lib/AST/GenericSignatureBuilder.cpp

@@ -2370,43 +2370,6 @@ Type ResolvedType::getDependentType(GenericSignatureBuilder &builder) const {
   return result->isTypeParameter() ? result : Type();
 }
 
-/// If there is a same-type requirement to be added for the given nested type
-/// due to a superclass constraint on the parent type, add it now.
-static void maybeAddSameTypeRequirementForNestedType(
-                                          ResolvedType nested,
-                                          const RequirementSource *superSource,
-                                          GenericSignatureBuilder &builder) {
-  // If there's no super conformance, we're done.
-  if (!superSource) return;
-
-  // If the nested type is already concrete, we're done.
-  if (nested.getAsConcreteType()) return;
-
-  // Dig out the associated type.
-  AssociatedTypeDecl *assocType = nullptr;
-  if (auto depMemTy =
-        nested.getDependentType(builder)->getAs<DependentMemberType>())
-    assocType = depMemTy->getAssocType();
-  else
-    return;
-
-  // Dig out the type witness.
-  auto superConformance = superSource->getProtocolConformance().getConcrete();
-  auto concreteType = superConformance->getTypeWitness(assocType);
-  if (!concreteType) return;
-
-  // We should only have interface types here.
-  assert(!superConformance->getType()->hasArchetype());
-  assert(!concreteType->hasArchetype());
-
-  // Add the same-type constraint.
-  auto nestedSource = superSource->viaParent(builder, assocType);
-
-  builder.addSameTypeRequirement(
-        nested.getUnresolvedType(), concreteType, nestedSource,
-        GenericSignatureBuilder::UnresolvedHandlingKind::GenerateConstraints);
-}
-
 auto PotentialArchetype::getOrCreateEquivalenceClass(
                                        GenericSignatureBuilder &builder) const
     -> EquivalenceClass * {
@@ -2542,7 +2505,9 @@ static void concretizeNestedTypeFromConcreteParent(
   // If we don't already have a conformance of the parent to this protocol,
   // add it now; it was elided earlier.
   if (parentEquiv->conformsTo.count(proto) == 0) {
-    auto source = parentEquiv->concreteTypeConstraints.front().source;
+    auto source = (!isSuperclassConstrained
+                   ? parentEquiv->concreteTypeConstraints.front().source
+                   : parentEquiv->superclassConstraints.front().source);
     parentEquiv->recordConformanceConstraint(builder, parent, proto, source);
   }
 
@@ -2572,7 +2537,7 @@ static void concretizeNestedTypeFromConcreteParent(
   if (conformance.isConcrete()) {
     witnessType =
       conformance.getConcrete()->getTypeWitness(assocType);
-    if (!witnessType || witnessType->hasError())
+    if (!witnessType)
       return; // FIXME: should we delay here?
   } else if (auto archetype = concreteParent->getAs<ArchetypeType>()) {
     witnessType = archetype->getNestedType(assocType->getName());
@@ -2637,20 +2602,11 @@ PotentialArchetype *PotentialArchetype::updateNestedTypeForConformance(
 
   // If we have a potential archetype that requires more processing, do so now.
   if (shouldUpdatePA) {
-    // If there's a superclass constraint that conforms to the protocol,
-    // add the appropriate same-type relationship.
-    const auto proto = assocType->getProtocol();
-    if (proto) {
-      if (auto superSource = builder.resolveSuperConformance(this, proto)) {
-        maybeAddSameTypeRequirementForNestedType(resultPA, superSource,
-                                                 builder);
-      }
-    }
-
     // We know something concrete about the parent PA, so we need to propagate
     // that information to this new archetype.
-    if (isConcreteType()) {
-      concretizeNestedTypeFromConcreteParent(this, resultPA, builder);
+    if (auto equivClass = getEquivalenceClassIfPresent()) {
+      if (equivClass->concreteType || equivClass->superclass)
+        concretizeNestedTypeFromConcreteParent(this, resultPA, builder);
     }
   }
 
@@ -3585,50 +3541,29 @@ static Type getStructuralType(TypeDecl *typeDecl, bool keepSugar) {
   return typeDecl->getDeclaredInterfaceType();
 }
 
-static Type substituteConcreteType(GenericSignatureBuilder &builder,
-                                   PotentialArchetype *basePA,
+static Type substituteConcreteType(Type parentType,
                                    TypeDecl *concreteDecl) {
+  if (parentType->is<ErrorType>())
+    return parentType;
+
   assert(concreteDecl);
 
   auto *dc = concreteDecl->getDeclContext();
-  auto *proto = dc->getSelfProtocolDecl();
 
   // Form an unsubstituted type referring to the given type declaration,
   // for use in an inferred same-type requirement.
   auto type = getStructuralType(concreteDecl, /*keepSugar=*/true);
 
-  SubstitutionMap subMap;
-  if (proto) {
-    // Substitute in the type of the current PotentialArchetype in
-    // place of 'Self' here.
-    auto parentType = basePA->getDependentType(builder.getGenericParams());
-
-    subMap = SubstitutionMap::getProtocolSubstitutions(
-        proto, parentType, ProtocolConformanceRef(proto));
-  } else {
-    // Substitute in the superclass type.
-    auto parentPA = basePA->getEquivalenceClassIfPresent();
-    auto parentType =
-        parentPA->concreteType ? parentPA->concreteType : parentPA->superclass;
-    auto parentDecl = parentType->getAnyNominal();
-
-    subMap = parentType->getContextSubstitutionMap(
-        parentDecl->getParentModule(), dc);
-  }
+  auto subMap = parentType->getContextSubstitutionMap(
+      dc->getParentModule(), dc);
 
   return type.subst(subMap);
-};
+}
 
 ResolvedType GenericSignatureBuilder::maybeResolveEquivalenceClass(
                                     Type type,
                                     ArchetypeResolutionKind resolutionKind,
                                     bool wantExactPotentialArchetype) {
-  // An error type is best modeled as an unresolved potential archetype, since
-  // there's no way to be sure what it is actually meant to be.
-  if (type->is<ErrorType>()) {
-    return ResolvedType::forUnresolved(nullptr);
-  }
-
   // The equivalence class of a generic type is known directly.
   if (auto genericParam = type->getAs<GenericTypeParamType>()) {
     unsigned index = GenericParamKey(genericParam).findIndexIn(
@@ -3650,8 +3585,11 @@ ResolvedType GenericSignatureBuilder::maybeResolveEquivalenceClass(
                                    wantExactPotentialArchetype);
     if (!resolvedBase) return resolvedBase;
     // If the base is concrete, so is this member.
-    if (resolvedBase.getAsConcreteType())
-      return ResolvedType::forConcrete(type);
+    if (auto parentType = resolvedBase.getAsConcreteType()) {
+      auto concreteType = substituteConcreteType(parentType,
+                                                 depMemTy->getAssocType());
+      return ResolvedType::forConcrete(concreteType);
+    }
 
     // Find the nested type declaration for this.
     auto baseEquivClass = resolvedBase.getEquivalenceClass(*this);
@@ -3668,59 +3606,84 @@ ResolvedType GenericSignatureBuilder::maybeResolveEquivalenceClass(
       basePA = baseEquivClass->members.front();
     }
 
-    AssociatedTypeDecl *nestedTypeDecl = nullptr;
     if (auto assocType = depMemTy->getAssocType()) {
       // Check whether this associated type references a protocol to which
-      // the base conforms. If not, it's unresolved.
-      if (baseEquivClass->conformsTo.find(assocType->getProtocol())
+      // the base conforms. If not, it's either concrete or unresolved.
+      auto *proto = assocType->getProtocol();
+      if (baseEquivClass->conformsTo.find(proto)
           == baseEquivClass->conformsTo.end()) {
-        if (!baseEquivClass->concreteType ||
-            !lookupConformance(type->getCanonicalType(),
-                               baseEquivClass->concreteType,
-                               assocType->getProtocol())) {
+        if (baseEquivClass->concreteType &&
+            lookupConformance(type->getCanonicalType(),
+                              baseEquivClass->concreteType,
+                              proto)) {
+          // Fall through
+        } else if (baseEquivClass->superclass &&
+                   lookupConformance(type->getCanonicalType(),
+                                     baseEquivClass->superclass,
+                                     proto)) {
+          // Fall through
+        } else {
           return ResolvedType::forUnresolved(baseEquivClass);
         }
+
+        // FIXME: Instead of falling through, we ought to return a concrete
+        // type here, but then we fail to update a nested PotentialArchetype
+        // if one happens to already exist. It would be cleaner if concrete
+        // types never had nested PotentialArchetypes.
       }
 
-      nestedTypeDecl = assocType;
+      auto nestedPA =
+        basePA->updateNestedTypeForConformance(*this, assocType,
+                                               resolutionKind);
+      if (!nestedPA)
+        return ResolvedType::forUnresolved(baseEquivClass);
+
+      // If base resolved to the anchor, then the nested potential archetype
+      // we found is the resolved potential archetype. Return it directly,
+      // so it doesn't need to be resolved again.
+      if (basePA == resolvedBase.getPotentialArchetypeIfKnown())
+        return ResolvedType(nestedPA);
+
+      // Compute the resolved dependent type to return.
+      Type resolvedBaseType = resolvedBase.getDependentType(*this);
+      Type resolvedMemberType =
+          DependentMemberType::get(resolvedBaseType, assocType);
+
+      return ResolvedType(resolvedMemberType,
+                          nestedPA->getOrCreateEquivalenceClass(*this));
     } else {
-      auto *typeAlias =
+      auto *concreteDecl =
           baseEquivClass->lookupNestedType(*this, depMemTy->getName());
 
-      if (!typeAlias)
+      if (!concreteDecl)
         return ResolvedType::forUnresolved(baseEquivClass);
 
-      auto type = substituteConcreteType(*this, basePA, typeAlias);
-      return maybeResolveEquivalenceClass(type, resolutionKind,
-                                          wantExactPotentialArchetype);
-    }
+      Type parentType;
+      auto *proto = concreteDecl->getDeclContext()->getSelfProtocolDecl();
+      if (!proto) {
+        parentType = (baseEquivClass->concreteType
+                      ? baseEquivClass->concreteType
+                      : baseEquivClass->superclass);
+      } else {
+        if (baseEquivClass->concreteType &&
+            lookupConformance(type->getCanonicalType(),
+                              baseEquivClass->concreteType,
+                              proto)) {
+          parentType = baseEquivClass->concreteType;
+        } else if (baseEquivClass->superclass &&
+                   lookupConformance(type->getCanonicalType(),
+                                     baseEquivClass->superclass,
+                                     proto)) {
+          parentType = baseEquivClass->superclass;
+        } else {
+          parentType = basePA->getDependentType(getGenericParams());
+        }
+      }
 
-    auto nestedPA =
-      basePA->updateNestedTypeForConformance(*this, nestedTypeDecl,
-                                             resolutionKind);
-    if (!nestedPA)
-      return ResolvedType::forUnresolved(baseEquivClass);
-
-    // If base resolved to the anchor, then the nested potential archetype
-    // we found is the resolved potential archetype. Return it directly,
-    // so it doesn't need to be resolved again.
-    if (basePA == resolvedBase.getPotentialArchetypeIfKnown())
-      return ResolvedType(nestedPA);
-
-    // Compute the resolved dependent type to return.
-    Type resolvedBaseType = resolvedBase.getDependentType(*this);
-    Type resolvedMemberType;
-    if (auto assocType = dyn_cast<AssociatedTypeDecl>(nestedTypeDecl)) {
-      resolvedMemberType =
-        DependentMemberType::get(resolvedBaseType, assocType);
-    } else {
-      // Note: strange case that might not even really be dependent.
-      resolvedMemberType =
-        DependentMemberType::get(resolvedBaseType, depMemTy->getName());
+      auto concreteType = substituteConcreteType(parentType, concreteDecl);
+      return maybeResolveEquivalenceClass(concreteType, resolutionKind,
+                                          wantExactPotentialArchetype);
     }
-
-    return ResolvedType(resolvedMemberType,
-                         nestedPA->getOrCreateEquivalenceClass(*this));
   }
 
   // If it's not a type parameter, it won't directly resolve to one.
@@ -5523,7 +5486,8 @@ GenericSignatureBuilder::finalize(SourceLoc loc,
       // Don't allow a generic parameter to be equivalent to a concrete type,
       // because then we don't actually have a parameter.
       auto equivClass = rep->getOrCreateEquivalenceClass(*this);
-      if (equivClass->concreteType) {
+      if (equivClass->concreteType &&
+          !equivClass->concreteType->is<ErrorType>()) {
         if (auto constraint = equivClass->findAnyConcreteConstraintAsWritten()){
           Impl->HadAnyError = true;
 

test/Constraints/same_types.swift

@@ -89,15 +89,14 @@ func test6<T: Barrable>(_ t: T) -> (Y, X) where T.Bar == Y {
 }
 
 func test7<T: Barrable>(_ t: T) -> (Y, X) where T.Bar == Y, T.Bar.Foo == X {
-	// expected-warning@-1{{redundant same-type constraint 'T.Bar.Foo' == 'X'}}
-        // expected-note@-2{{same-type constraint 'T.Bar.Foo' == 'Y.Foo' (aka 'X') implied here}}
+	// expected-warning@-1{{neither type in same-type constraint ('Y.Foo' (aka 'X') or 'X') refers to a generic parameter or associated type}}
   return (t.bar, t.bar.foo)
 }
 
 func fail4<T: Barrable>(_ t: T) -> (Y, Z)
   where
-  T.Bar == Y, // expected-note{{same-type constraint 'T.Bar.Foo' == 'Y.Foo' (aka 'X') implied here}}
-  T.Bar.Foo == Z { // expected-error{{'T.Bar.Foo' cannot be equal to both 'Z' and 'Y.Foo' (aka 'X')}}
+  T.Bar == Y,
+  T.Bar.Foo == Z { // expected-error{{generic signature requires types 'Y.Foo' (aka 'X') and 'Z' to be the same}}
   return (t.bar, t.bar.foo) // expected-error{{cannot convert return expression of type '(Y, X)' to return type '(Y, Z)'}}
 }
 

test/Generics/Inputs/sr8945-other.swift

@@ -0,0 +1,3 @@
+public protocol P {
+  associatedtype T
+}

test/Generics/sr8945.swift

@@ -0,0 +1,17 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module %S/Inputs/sr8945-other.swift -emit-module-path %t/other.swiftmodule -module-name other
+// RUN: %target-swift-frontend -emit-silgen %s -I%t
+
+import other
+
+public class C : P {
+  public typealias T = Int
+}
+
+public func takesInt(_: Int) {}
+
+public func foo<T : C, S : Sequence>(_: T, _ xs: S) where S.Element == T.T {
+  for x in xs {
+    takesInt(x)
+  }
+}

test/IDE/print_ast_tc_decls_errors.swift

@@ -192,7 +192,7 @@ protocol AssociatedType1 {
 // TYREPR: {{^}}  associatedtype AssociatedTypeDecl4 : FooNonExistentProtocol, BarNonExistentProtocol{{$}}
 
   associatedtype AssociatedTypeDecl5 : FooClass
-// CHECK: {{^}}  associatedtype AssociatedTypeDecl5 : FooClass{{$}}
+// CHECK: {{^}}  associatedtype AssociatedTypeDecl5{{$}}
 }
 
 //===---

test/decl/protocol/req/recursion.swift

@@ -48,7 +48,7 @@ public struct S<A: P> where A.T == S<A> { // expected-error {{circular reference
 // expected-error@-2 {{generic struct 'S' references itself}}
   func f(a: A.T) {
     g(a: id(t: a))
-    // expected-error@-1 {{cannot convert value of type 'A.T' to expected argument type 'S<A>'}}
+    // expected-error@-1 {{type of expression is ambiguous without more context}}
     _ = A.T.self
   }
 

validation-test/compiler_crashers_2_fixed/0159-rdar40009245.swift

@@ -2,6 +2,7 @@
 
 protocol P {
     associatedtype A : P where A.X == Self
+    // expected-error@-1{{'X' is not a member type of 'Self.A}}
     associatedtype X : P where P.A == Self
     // expected-error@-1{{associated type 'A' can only be used with a concrete type or generic parameter base}}
 }

validation-test/compiler_crashers_2_fixed/0163-sr8033.swift

@@ -7,3 +7,4 @@ protocol P1 {
 }
 extension Foo: P1 where A : P1 {} // expected-error {{unsupported recursion for reference to associated type 'A' of type 'Foo<T>'}}
 // expected-error@-1 {{type 'Foo<T>' does not conform to protocol 'P1'}}
+// expected-error@-2 {{type 'Foo<T>' in conformance requirement does not refer to a generic parameter or associated type}}