[Archetype builder] Cope with typo-corrected nested types slightly better.

Fix two issues that cropped up in rdar://problem/29261689:

* We didn't have an ordering between two typo-corrected nested types
  that were corrected to the same name, triggering an assertion, which
  we resolve by ordering based on the original (pre-correction) names.
* In some cases, typo corrections would not be diagnosed because the
  type checker wasn't able to map them to specific uses of that
  type. Introduce a lame fallback case to make sure we get some
  diagnostic---even though the location information is poor.

All of this needs an overhaul, but not today.

Fixes rdar://problem/29261689.

Author: DougGregor

include/swift/AST/ArchetypeBuilder.h

@@ -291,6 +291,11 @@ class ArchetypeBuilder {
   /// be made concrete.
   void finalize(SourceLoc loc, bool allowConcreteGenericParams=false);
 
+  /// Diagnose any remaining renames.
+  ///
+  /// \returns \c true if there were any remaining renames to diagnose.
+  bool diagnoseRemainingRenames(SourceLoc loc);
+
   /// \brief Resolve the given type to the potential archetype it names.
   ///
   /// This routine will synthesize nested types as required to refer to a
@@ -409,8 +414,12 @@ class ArchetypeBuilder::PotentialArchetype {
   /// the superclass type.
   unsigned RecursiveSuperclassType : 1;
 
-  /// Whether we have renamed this (nested) type due to typo correction.
-  unsigned Renamed : 1;
+  /// Whether we have diagnosed a rename.
+  unsigned DiagnosedRename : 1;
+
+  /// If we have renamed this (nested) type due to typo correction,
+  /// the old name.
+  Identifier OrigName;
 
   /// The equivalence class of this potential archetype.
   llvm::TinyPtrVector<PotentialArchetype *> EquivalenceClass;
@@ -421,7 +430,7 @@ class ArchetypeBuilder::PotentialArchetype {
     : ParentOrParam(Parent), NameOrAssociatedType(Name), Representative(this),
       IsRecursive(false), Invalid(false), SubstitutingConcreteType(false),
       RecursiveConcreteType(false), RecursiveSuperclassType(false),
-      Renamed(false)
+      DiagnosedRename(false)
   { 
     assert(Parent != nullptr && "Not an associated type?");
     EquivalenceClass.push_back(this);
@@ -432,8 +441,8 @@ class ArchetypeBuilder::PotentialArchetype {
     : ParentOrParam(Parent), NameOrAssociatedType(AssocType), 
       Representative(this), IsRecursive(false), Invalid(false),
       SubstitutingConcreteType(false), RecursiveConcreteType(false),
-      RecursiveSuperclassType(false), Renamed(false)
-  { 
+      RecursiveSuperclassType(false), DiagnosedRename(false)
+  {
     assert(Parent != nullptr && "Not an associated type?");
     EquivalenceClass.push_back(this);
   }
@@ -443,7 +452,7 @@ class ArchetypeBuilder::PotentialArchetype {
     : ParentOrParam(Parent), NameOrAssociatedType(TypeAlias),
       Representative(this), IsRecursive(false), Invalid(false),
       SubstitutingConcreteType(false), RecursiveConcreteType(false),
-      RecursiveSuperclassType(false), Renamed(false)
+      RecursiveSuperclassType(false), DiagnosedRename(false)
   {
     assert(Parent != nullptr && "Not an associated type?");
     EquivalenceClass.push_back(this);
@@ -457,7 +466,7 @@ class ArchetypeBuilder::PotentialArchetype {
       NameOrAssociatedType(Name), Representative(this), IsRecursive(false),
       Invalid(false), SubstitutingConcreteType(false),
       RecursiveConcreteType(false), RecursiveSuperclassType(false),
-      Renamed(false)
+      DiagnosedRename(false)
   {
     EquivalenceClass.push_back(this);
   }
@@ -601,15 +610,27 @@ class ArchetypeBuilder::PotentialArchetype {
 
   /// Determine whether this archetype was renamed due to typo
   /// correction. If so, \c getName() retrieves the new name.
-  bool wasRenamed() const { return Renamed; }
+  bool wasRenamed() const { return !OrigName.empty(); }
 
   /// Note that this potential archetype was renamed (due to typo
   /// correction), providing the new name.
   void setRenamed(Identifier newName) {
+    OrigName = getName();
     NameOrAssociatedType = newName;
-    Renamed = true;
   }
 
+  /// For a renamed potential archetype, retrieve the original name.
+  Identifier getOriginalName() const {
+    assert(wasRenamed());
+    return OrigName;
+  }
+
+  /// Whether we already diagnosed this rename.
+  bool alreadyDiagnosedRename() const { return DiagnosedRename; }
+
+  /// Note that we already diagnosed this rename.
+  void setAlreadyDiagnosedRename() { DiagnosedRename = true; }
+
   /// Whether this potential archetype makes a better archetype anchor than
   /// the given archetype anchor.
   bool isBetterArchetypeAnchor(PotentialArchetype *other) const;

lib/AST/ArchetypeBuilder.cpp

@@ -127,6 +127,9 @@ struct ArchetypeBuilder::Implementation {
   /// Once all requirements have been added, this will be zero in well-formed
   /// code.
   unsigned NumUnresolvedNestedTypes = 0;
+
+  /// The nested types that have been renamed.
+  SmallVector<PotentialArchetype *, 4> RenamedNestedTypes;
 };
 
 ArchetypeBuilder::PotentialArchetype::~PotentialArchetype() {
@@ -1146,6 +1149,17 @@ static int compareDependentTypes(ArchetypeBuilder::PotentialArchetype * const* p
   if (b->getResolvedAssociatedType())
     return +1;
 
+  // Along the error path where one or both of the potential archetypes was
+  // renamed due to typo correction,
+  if (a->wasRenamed() || b->wasRenamed()) {
+    if (a->wasRenamed() != b->wasRenamed())
+      return a->wasRenamed() ? +1 : -1;
+
+    if (int compareNames = a->getOriginalName().str().compare(
+                                                    b->getOriginalName().str()))
+      return compareNames;
+  }
+
   llvm_unreachable("potential archetype total order failure");
 }
 
@@ -1846,6 +1860,7 @@ ArchetypeBuilder::finalize(SourceLoc loc, bool allowConcreteGenericParams) {
 
       // Set the typo-corrected name.
       pa->setRenamed(correction);
+      Impl->RenamedNestedTypes.push_back(pa);
 
       // Resolve the associated type and merge the potential archetypes.
       auto replacement = pa->getParent()->getNestedType(correction, *this);
@@ -1856,6 +1871,20 @@ ArchetypeBuilder::finalize(SourceLoc loc, bool allowConcreteGenericParams) {
   }
 }
 
+bool ArchetypeBuilder::diagnoseRemainingRenames(SourceLoc loc) {
+  bool invalid = false;
+  for (auto pa : Impl->RenamedNestedTypes) {
+    if (pa->alreadyDiagnosedRename()) continue;
+
+    Diags.diagnose(loc, diag::invalid_member_type_suggest,
+                   pa->getParent()->getDependentType(*this, true),
+                   pa->getOriginalName(), pa->getName());
+    invalid = true;
+  }
+
+  return invalid;
+}
+
 template<typename F>
 void ArchetypeBuilder::visitPotentialArchetypes(F f) {
   // Stack containing all of the potential archetypes to visit.

lib/Sema/TypeCheckGeneric.cpp

@@ -191,7 +191,8 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType(
                 baseTy, ref->getIdentifier(), newName)
       .fixItReplace(ref->getIdLoc(), newName.str());
     ref->overwriteIdentifier(newName);
-
+    nestedPA->setAlreadyDiagnosedRename();
+    
     // Go get the actual nested type.
     nestedPA = basePA->getNestedType(newName, Builder);
     assert(!nestedPA->wasRenamed());
@@ -485,6 +486,8 @@ TypeChecker::validateGenericFuncSignature(AbstractFunctionDecl *func) {
   CompleteGenericTypeResolver completeResolver(*this, builder);
   if (checkGenericFuncSignature(*this, nullptr, func, completeResolver))
     invalid = true;
+  if (builder.diagnoseRemainingRenames(func->getLoc()))
+    invalid = true;
 
   // The generic function signature is complete and well-formed. Determine
   // the type of the generic function.
@@ -683,6 +686,7 @@ GenericSignature *TypeChecker::validateGenericSignature(
   CompleteGenericTypeResolver completeResolver(*this, builder);
   checkGenericParamList(nullptr, genericParams, nullptr,
                         nullptr, &completeResolver);
+  (void)builder.diagnoseRemainingRenames(genericParams->getSourceRange().Start);
 
   // Record the generic type parameter types and the requirements.
   auto sig = builder.getGenericSignature();

test/Generics/associated_type_typo.swift

@@ -24,6 +24,7 @@ func typoAssoc2<T : P1, U : P1>() where T.assoc == U.assoc {}
 // CHECK-GENERIC-LABEL: .typoAssoc2
 // CHECK-GENERIC: Generic signature: <T, U where T : P1, U : P1, T.Assoc == U.Assoc>
 
+// expected-error@+3{{'U.AssocP2' does not have a member type named 'assoc'; did you mean 'Assoc'?}}
 // expected-error@+3{{'T.AssocP2' does not have a member type named 'assoc'; did you mean 'Assoc'?}}{{42-47=Assoc}}
 // expected-error@+2{{'U.AssocP2' does not have a member type named 'assoc'; did you mean 'Assoc'?}}{{19-24=Assoc}}
 func typoAssoc3<T : P2, U : P2>()
@@ -57,3 +58,25 @@ func typoFunc2<T : P1>(x: TypoType, y: T) { // expected-error{{use of undeclared
 
 func typoFunc3<T : P1>(x: TypoType, y: (T.Assoc) -> ()) { // expected-error{{use of undeclared type 'TypoType'}}
 }
+
+// rdar://problem/29261689
+typealias Element_<S: Sequence> = S.Iterator.Element
+
+public protocol _Indexable1 {
+  associatedtype Slice
+}
+public protocol Indexable : _Indexable1 {
+  associatedtype Slice : _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'?}}
+  func matched<C: Indexable>(atStartOf c: C)
+  where Element_<C> == Element
+  , Element_<C.Slice> == Element
+}