[Archetype builder] Make canonicalization of dependent types deterministic.

The canonicalization of dependent member types had some
nondeterminism. The root of the problem was that we couldn't
round-trip dependent member types through the archetype
builder---resolving them to a potential archetype lost the specific
associated type that was recorded in the dependent member type, which
affected canonicalization. Maintain that information, make sure that
we always get the right archetype anchor, and tighten up the
canonicalization logic within a generic signature.

Fixes rdar://problem/30274260 and should unblock some other work
that depends on sanity from the archetype builder and generic
signature canonicalization.

include/swift/AST/ArchetypeBuilder.h

@@ -570,7 +570,7 @@ public:
 
   /// \brief Retrieve the potential archetype to be used as the anchor for
   /// potential archetype computations.
-  PotentialArchetype *getArchetypeAnchor();
+  PotentialArchetype *getArchetypeAnchor(ArchetypeBuilder &builder);
 
   /// Add a same-type constraint between this archetype and the given
   /// other archetype.

lib/AST/ArchetypeBuilder.cpp

@@ -479,8 +479,31 @@ static bool isBetterArchetypeAnchor(
   return compareDependentTypes(&mutablePA, &mutablePB) < 0;
 }
 
-auto ArchetypeBuilder::PotentialArchetype::getArchetypeAnchor()
+/// Rebuild the given potential archetype based on anchors.
+static ArchetypeBuilder::PotentialArchetype*rebuildPotentialArchetypeAnchor(
+                                    ArchetypeBuilder::PotentialArchetype *pa,
+                                    ArchetypeBuilder &builder) {
+  if (auto parent = pa->getParent()) {
+    auto parentAnchor =
+      rebuildPotentialArchetypeAnchor(parent->getArchetypeAnchor(builder),
+                                      builder);
+    if (parent == parentAnchor) return pa;
+
+    if (auto assocType = pa->getResolvedAssociatedType())
+      return parentAnchor->getNestedType(assocType, builder);
+
+    return parentAnchor->getNestedType(pa->getName(), builder);
+  }
+
+  return pa;
+}
+
+auto ArchetypeBuilder::PotentialArchetype::getArchetypeAnchor(
+                                                      ArchetypeBuilder &builder)
        -> PotentialArchetype * {
+  // Rebuild the potential archetype anchor for this type, so the equivalence
+  // class will contain the anchor.
+  (void)rebuildPotentialArchetypeAnchor(this, builder);
 
   // Find the best archetype within this equivalence class.
   PotentialArchetype *rep = getRepresentative();
@@ -523,7 +546,9 @@ auto ArchetypeBuilder::PotentialArchetype::getNestedType(
   // Attempt to resolve this nested type to an associated type
   // of one of the protocols to which the parent potential
   // archetype conforms.
-  for (auto &conforms : getRepresentative()->ConformsTo) {
+  SmallVector<std::pair<ProtocolDecl *, RequirementSource>, 4>
+    conformsTo(rep->ConformsTo.begin(), rep->ConformsTo.end());
+  for (auto &conforms : conformsTo) {
     for (auto member : conforms.first->lookupDirect(nestedName)) {
       PotentialArchetype *pa;
       
@@ -606,7 +631,25 @@ auto ArchetypeBuilder::PotentialArchetype::getNestedType(
 auto ArchetypeBuilder::PotentialArchetype::getNestedType(
                             AssociatedTypeDecl *assocType,
                             ArchetypeBuilder &builder) -> PotentialArchetype * {
-  return getNestedType(assocType->getName(), builder);
+  // Add the requirement that this type conform to the protocol of the
+  // associated type. We treat this as "inferred" because it comes from the
+  // structure of the type---there will be an explicit or implied requirement
+  // somewhere else.
+  bool failed = builder.addConformanceRequirement(
+                  this, assocType->getProtocol(),
+                  RequirementSource(RequirementSource::Inferred, SourceLoc()));
+  (void)failed;
+
+  // Trigger the construction of nested types with this name.
+  auto fallback = getNestedType(assocType->getName(), builder);
+
+  // Find the nested type that resolved to this associated type.
+  for (const auto &nested : NestedTypes[assocType->getName()]) {
+    if (nested->getResolvedAssociatedType() == assocType) return nested;
+  }
+
+  assert(failed && "unable to find nested type that we know is there");
+  return fallback;
 }
 
 Type ArchetypeBuilder::PotentialArchetype::getTypeInContext(
@@ -618,7 +661,7 @@ Type ArchetypeBuilder::PotentialArchetype::getTypeInContext(
   // Retrieve the archetype from the archetype anchor in this equivalence class.
   // The anchor must not have any concrete parents (otherwise we would just
   // use the representative).
-  auto archetypeAnchor = getArchetypeAnchor();
+  auto archetypeAnchor = getArchetypeAnchor(builder);
   if (archetypeAnchor != this)
     return archetypeAnchor->getTypeInContext(builder, genericEnv);
 
@@ -2128,7 +2171,14 @@ void ArchetypeBuilder::enumerateRequirements(llvm::function_ref<
                            PotentialArchetype *archetype,
                            ArchetypeBuilder::RequirementRHS constraint,
                            RequirementSource source)> f) {
-  // First, collect all archetypes, and sort them.
+   // Create anchors for all of the potential archetypes.
+   // FIXME: This is because we might be missing some from the equivalence
+   // classes.
+   visitPotentialArchetypes([&](PotentialArchetype *archetype) {
+     archetype->getArchetypeAnchor(*this);
+   });
+
+  // Collect all archetypes, and sort them.
   SmallVector<PotentialArchetype *, 8> archetypes;
   visitPotentialArchetypes([&](PotentialArchetype *archetype) {
     archetypes.push_back(archetype);
@@ -2198,7 +2248,7 @@ void ArchetypeBuilder::enumerateRequirements(llvm::function_ref<
     };
 
     // If this is not the archetype anchor, we're done.
-    if (archetype != archetype->getArchetypeAnchor())
+    if (archetype != archetype->getArchetypeAnchor(*this))
       continue;
 
     // If we have a superclass, produce a superclass requirement
@@ -2278,6 +2328,12 @@ void ArchetypeBuilder::dump(llvm::raw_ostream &out) {
     }
   });
   out << "\n";
+
+  out << "Potential archetypes:\n";
+  for (auto pa : Impl->PotentialArchetypes) {
+    pa->dump(out, &Context.SourceMgr, 2);
+  }
+  out << "\n";
 }
 
 void ArchetypeBuilder::addGenericSignature(GenericSignature *sig) {

lib/AST/GenericSignature.cpp

@@ -584,10 +584,6 @@ bool GenericSignature::isCanonicalTypeInContext(Type type,
   if (!type->hasTypeParameter())
     return true;
 
-#if false
-  // FIXME: This is broken because resolveArchetype() doesn't take the
-  // actual associated types into account.
-
   // Look for non-canonical type parameters.
   return !type.findIf([&](Type component) -> bool {
     if (!component->isTypeParameter()) return false;
@@ -595,12 +591,9 @@ bool GenericSignature::isCanonicalTypeInContext(Type type,
     auto pa = builder.resolveArchetype(component);
     if (!pa) return false;
 
-    auto rep = pa->getArchetypeAnchor();
+    auto rep = pa->getArchetypeAnchor(builder);
     return (rep->isConcreteType() || pa != rep);
   });
-#else
-  return getCanonicalTypeInContext(type, builder)->isEqual(type);
-#endif
 }
 
 CanType GenericSignature::getCanonicalTypeInContext(Type type,
@@ -623,7 +616,7 @@ CanType GenericSignature::getCanonicalTypeInContext(Type type,
     auto pa = builder.resolveArchetype(Type(component));
     if (!pa) return None;
 
-    auto rep = pa->getArchetypeAnchor();
+    auto rep = pa->getArchetypeAnchor(builder);
     if (rep->isConcreteType()) {
       return getCanonicalTypeInContext(rep->getConcreteType(), builder);
     }
@@ -632,12 +625,8 @@ CanType GenericSignature::getCanonicalTypeInContext(Type type,
   });
   
   auto result = type->getCanonicalType();
-#if false
-  // FIXME: Bring this back when isCanonicalTypeInContext() doesn't call
-  // getCanonicalTypeInContext().
-  assert(isCanonicalTypeInContext(result, builder));
-#endif
 
+  assert(isCanonicalTypeInContext(result, builder));
   return result;
 }
 

lib/Sema/GenericTypeResolver.h

@@ -87,6 +87,10 @@ public:
   /// \returns the type of the declaration in context..
   virtual Type resolveTypeOfDecl(TypeDecl *decl) = 0;
 
+  /// Determine whether the given types are equivalent within the generic
+  /// context.
+  virtual bool areSameType(Type type1, Type type2) = 0;
+
   /// Set the contextual type or the interface type of the parameter.
   virtual void recordParamType(ParamDecl *decl, Type ty) = 0;
 };
@@ -116,6 +120,8 @@ public:
 
   virtual Type resolveTypeOfDecl(TypeDecl *decl);
 
+  virtual bool areSameType(Type type1, Type type2);
+
   virtual void recordParamType(ParamDecl *decl, Type ty);
 };
 
@@ -148,6 +154,8 @@ public:
 
   virtual Type resolveTypeOfDecl(TypeDecl *decl);
 
+  virtual bool areSameType(Type type1, Type type2);
+
   virtual void recordParamType(ParamDecl *decl, Type ty);
 };
 
@@ -180,6 +188,8 @@ public:
 
   virtual Type resolveTypeOfDecl(TypeDecl *decl);
 
+  virtual bool areSameType(Type type1, Type type2);
+
   virtual void recordParamType(ParamDecl *decl, Type ty);
 };
 

lib/Sema/TypeCheckGeneric.cpp

@@ -68,6 +68,18 @@ Type DependentGenericTypeResolver::resolveTypeOfDecl(TypeDecl *decl) {
   return decl->getDeclaredInterfaceType();
 }
 
+bool DependentGenericTypeResolver::areSameType(Type type1, Type type2) {
+  if (!type1->hasTypeParameter() && !type2->hasTypeParameter())
+    return type1->isEqual(type2);
+
+  auto pa1 = Builder.resolveArchetype(type1);
+  auto pa2 = Builder.resolveArchetype(type2);
+  if (pa1 && pa2)
+    return pa1->getArchetypeAnchor(Builder) == pa2->getArchetypeAnchor(Builder);
+
+  return type1->isEqual(type2);
+}
+
 void DependentGenericTypeResolver::recordParamType(ParamDecl *decl, Type type) {
   // Do nothing
 }
@@ -110,6 +122,10 @@ Type GenericTypeToArchetypeResolver::resolveTypeOfDecl(TypeDecl *decl) {
       decl->getDeclaredInterfaceType());
 }
 
+bool GenericTypeToArchetypeResolver::areSameType(Type type1, Type type2) {
+  return type1->isEqual(type2);
+}
+
 void GenericTypeToArchetypeResolver::recordParamType(ParamDecl *decl, Type type) {
   decl->setType(type);
 
@@ -220,6 +236,18 @@ Type CompleteGenericTypeResolver::resolveTypeOfDecl(TypeDecl *decl) {
   return decl->getDeclaredInterfaceType();
 }
 
+bool CompleteGenericTypeResolver::areSameType(Type type1, Type type2) {
+  if (!type1->hasTypeParameter() && !type2->hasTypeParameter())
+    return type1->isEqual(type2);
+
+  auto pa1 = Builder.resolveArchetype(type1);
+  auto pa2 = Builder.resolveArchetype(type2);
+  if (pa1 && pa2)
+    return pa1->getArchetypeAnchor(Builder) == pa2->getArchetypeAnchor(Builder);
+
+  return type1->isEqual(type2);
+}
+
 void
 CompleteGenericTypeResolver::recordParamType(ParamDecl *decl, Type type) {
   decl->setInterfaceType(type);

lib/Sema/TypeCheckType.cpp

@@ -1095,7 +1095,7 @@ resolveTopLevelIdentTypeComponent(TypeChecker &TC, DeclContext *DC,
     }
 
     // Otherwise, check for an ambiguity.
-    if (!current->isEqual(type)) {
+    if (!resolver->areSameType(current, type)) {
       isAmbiguous = true;
       break;
     }

test/Generics/associated_type_typo.swift

@@ -37,10 +37,10 @@ func typoAssoc4<T : P2>(_: T) where T.Assocp2.assoc : P3 {}
 
 // CHECK-GENERIC-LABEL: .typoAssoc4@
 // CHECK-GENERIC-NEXT: Requirements:
-// CHECK-GENERIC-NEXT:   T : P2 [explicit
+// CHECK-GENERIC-NEXT:   T : P2 [inferred]
 // CHECK-GENERIC-NEXT:   T[.P2].AssocP2 : P1 [protocol
 // CHECK-GENERIC-NEXT:   T[.P2].AssocP2[.P1].Assoc : P3 [explicit
-// CHECK-GENERIC-NEXT: Generic signature
+// CHECK-GENERIC-NEXT: Potential archetypes
 
 // <rdar://problem/19620340>
 
@@ -67,11 +67,9 @@ public protocol Indexable : _Indexable1 {
 
 protocol Pattern {
   associatedtype Element : Equatable
-  
-  // FIXME: Diagnostics here are very poor
-  // expected-error@+3{{'C' does not have a member type named 'Iterator'; did you mean 'Slice'?}}
-  // expected-error@+2{{'C.Slice' does not have a member type named 'Element'; did you mean 'Slice'?}}
-  // expected-error@+1{{'C.Slice' does not have a member type named 'Iterator'; did you mean 'Slice'?}}
+
+  // FIXME: This works for all of the wrong reasons, but it is correct that
+  // it works.
   func matched<C: Indexable>(atStartOf c: C)
   where Element_<C> == Element
   , Element_<C.Slice> == Element

test/Generics/requirement_inference.swift

@@ -119,8 +119,8 @@ func inferSameType1<T, U>(_ x: Model_P3_P4_Eq<T, U>) { }
 
 // CHECK-LABEL: .inferSameType2@
 // CHECK-NEXT: Requirements:
-// CHECK-NEXT:   T : P3 [explicit @ {{.*}}requirement_inference.swift:{{.*}}:25]
-// CHECK-NEXT:   U : P4 [explicit @ {{.*}}requirement_inference.swift:{{.*}}:33]
+// CHECK-NEXT:   T : P3 [inferred]
+// CHECK-NEXT:   U : P4 [inferred]
 // CHECK-NEXT:   T[.P3].P3Assoc : P1 [redundant @ {{.*}}requirement_inference.swift:{{.*}}:18]
 // CHECK-NEXT:   T[.P3].P3Assoc : P2 [protocol @ {{.*}}requirement_inference.swift:{{.*}}:18]
 // CHECK-NEXT:   T[.P3].P3Assoc == U[.P4].P4Assoc [explicit]
@@ -128,10 +128,10 @@ func inferSameType2<T : P3, U : P4>(_: T, _: U) where U.P4Assoc : P2, T.P3Assoc
 
 // CHECK-LABEL: .inferSameType3@
 // CHECK-NEXT: Requirements:
-// CHECK-NEXT:   T : PCommonAssoc1 [explicit @ {{.*}}requirement_inference.swift:{{.*}}:25]
-// CHECK-NEXT:   T : PCommonAssoc2 [explicit @ {{.*}}requirement_inference.swift:{{.*}}:76]
+// CHECK-NEXT:   T : PCommonAssoc1 [inferred]
+// CHECK-NEXT:   T : PCommonAssoc2 [inferred]
 // CHECK-NEXT:   T[.PCommonAssoc1].CommonAssoc : P1 [explicit @ {{.*}}requirement_inference.swift:{{.*}}:68]
-// CHECK-NEXT: Generic signature
+// CHECK-NEXT: Potential archetypes
 func inferSameType3<T : PCommonAssoc1>(_: T) where T.CommonAssoc : P1, T : PCommonAssoc2 {}
 
 protocol P5 {

test/decl/protocol/recursive_requirement.swift

@@ -30,7 +30,7 @@ protocol PP2 {
 }
 
 protocol P2 : PP2 {
-  associatedtype A = Self
+  associatedtype A = Self // expected-error{{type may not reference itself as a requirement}}
 }
 
 struct X2<T: P2> {