Don't drop requirements from potential archetypes if they came from a child

It was possible to create a same-type requirement between a type and
one of its associated types. If one of those associated types had
inferred conformance constraints, we would drop them from the parent
type.

This is not actually correct because then we have no way to recover
metadata for the parent type.

The underlying problem has been present for a long time, however this
particular failure mode was new to Swift 3, when we stopped passing
metadata for associated types.

Fixes <rdar://problem/27018457>.

Author: slavapestov

include/swift/AST/ArchetypeBuilder.h

@@ -552,7 +552,8 @@ class ArchetypeBuilder::PotentialArchetype {
   /// Add a conformance to this potential archetype.
   ///
   /// \returns true if the conformance was new, false if it already existed.
-  bool addConformance(ProtocolDecl *proto, const RequirementSource &source,
+  bool addConformance(ProtocolDecl *proto, bool updateExistingSource,
+                      const RequirementSource &source,
                       ArchetypeBuilder &builder);
 
   /// Retrieve the superclass of this archetype.

lib/AST/ArchetypeBuilder.cpp

@@ -322,18 +322,20 @@ static void maybeAddSameTypeRequirementForNestedType(
 
 bool ArchetypeBuilder::PotentialArchetype::addConformance(
        ProtocolDecl *proto, 
+       bool updateExistingSource,
        const RequirementSource &source,
        ArchetypeBuilder &builder) {
   auto rep = getRepresentative();
   if (rep != this)
-    return rep->addConformance(proto, source, builder);
+    return rep->addConformance(proto, updateExistingSource, source, builder);
 
   // Check whether we already know about this conformance.
   auto known = ConformsTo.find(proto);
   if (known != ConformsTo.end()) {
     // We already have this requirement. Update the requirement source
     // appropriately.
-    updateRequirementSource(known->second, source);
+    if (updateExistingSource)
+      updateRequirementSource(known->second, source);
     return false;
   }
 
@@ -963,7 +965,7 @@ bool ArchetypeBuilder::addConformanceRequirement(PotentialArchetype *PAT,
   auto T = PAT->getRepresentative();
 
   // Add the requirement, if we haven't done so already.
-  if (!T->addConformance(Proto, Source, *this))
+  if (!T->addConformance(Proto, /*updateExistingSource=*/true, Source, *this))
     return false;
 
   RequirementSource InnerSource(RequirementSource::Protocol, Source.getLoc());
@@ -1159,7 +1161,20 @@ bool ArchetypeBuilder::addSameTypeRequirementBetweenArchetypes(
     T1->ArchetypeOrConcreteType = NestedType::forConcreteType(concrete2);
     T1->SameTypeSource = T2->SameTypeSource;
   }
-  
+
+  // Don't mark requirements as redundant if they come from one of our
+  // child archetypes. This is a targeted fix -- more general cases
+  // continue to break. In general, we need to detect cycles in the
+  // archetype graph and not propagate requirement source information
+  // along back edges.
+  bool creatingCycle = false;
+  auto T2Parent = T2;
+  while(T2Parent != nullptr) {
+    if (T2Parent->getRepresentative() == T1)
+      creatingCycle = true;
+    T2Parent = T2Parent->getParent();
+  }
+
   // Make T1 the representative of T2, merging the equivalence classes.
   T2->Representative = T1;
   T2->SameTypeSource = Source;
@@ -1175,7 +1190,8 @@ bool ArchetypeBuilder::addSameTypeRequirementBetweenArchetypes(
 
   // Add all of the protocol conformance requirements of T2 to T1.
   for (auto conforms : T2->ConformsTo) {
-    T1->addConformance(conforms.first, conforms.second, *this);
+    T1->addConformance(conforms.first, /*updateExistingSource=*/!creatingCycle,
+                       conforms.second, *this);
   }
 
   // Recursively merge the associated types of T2 into T1.
@@ -1331,7 +1347,8 @@ bool ArchetypeBuilder::addAbstractTypeParamRequirements(
     assocType->setInvalid();
 
     // FIXME: Drop this protocol.
-    pa->addConformance(proto, RequirementSource(kind, loc), *this);
+    pa->addConformance(proto, /*updateExistingSource=*/true,
+                       RequirementSource(kind, loc), *this);
   };
 
   // If the abstract type parameter already has an archetype assigned,

test/IRGen/same_type_constraints.swift

@@ -42,3 +42,19 @@ public class GenericKlazz<T: DataType, R: E> : E where R.Data == T
      d = Dict()
   }
 }
+
+// This used to hit an infinite loop - <rdar://problem/27018457>
+public protocol CodingType {
+    associatedtype ValueType
+}
+
+public protocol ValueCoding {
+    associatedtype Coder: CodingType
+}
+
+func foo<Self>(s: Self)
+where Self : CodingType,
+      Self.ValueType: ValueCoding,
+      Self.ValueType.Coder == Self {
+  print(Self.ValueType.self)
+}

test/decl/protocol/protocols.swift

@@ -235,7 +235,7 @@ struct WrongIsEqual : IsEqualComparable { // expected-error{{type 'WrongIsEqual'
 //===----------------------------------------------------------------------===//
 
 func existentialSequence(_ e: Sequence) { // expected-error{{has Self or associated type requirements}}
-  var x = e.makeIterator() // expected-error{{'Sequence' is not convertible to '<<error type>>'}}
+  var x = e.makeIterator() // expected-error{{type 'Sequence' does not conform to protocol 'IteratorProtocol'}}
   x.next()
   x.nonexistent()
 }