AST: Better checks for same-type constraints making generic parameters concrete

The old logic missed some cases where this could come up.

include/swift/AST/ArchetypeBuilder.h

@@ -305,8 +305,11 @@ public:
   /// Finalize the set of requirements, performing any remaining checking
   /// required before generating archetypes.
   ///
+  /// \param allowConcreteGenericParams If true, allow generic parameters to
+  /// be made concrete.
+  ///
   /// \returns true if an error occurs, false otherwise.
-  bool finalize(SourceLoc loc);
+  bool finalize(SourceLoc loc, bool allowConcreteGenericParams=false);
 
   /// \brief Resolve the given type to the potential archetype it names.
   ///

lib/AST/ArchetypeBuilder.cpp

@@ -1155,15 +1155,6 @@ bool ArchetypeBuilder::addSameTypeRequirementBetweenArchetypes(
        compareDependentTypes(&T2, &T1) < 0))
     std::swap(T1, T2);
 
-  // Don't allow two generic parameters to be equivalent, because then we
-  // don't actually have two parameters.
-  // FIXME: Should we simply allow this?
-  if (T1Depth == 0 && T2Depth == 0) {
-    Diags.diagnose(Source.getLoc(), diag::requires_generic_params_made_equal,
-                   T1->getName(), T2->getName());
-    return true;
-  }
-  
   // Merge any concrete constraints.
   Type concrete1 = T1->ArchetypeOrConcreteType.getAsConcreteType();
   Type concrete2 = T2->ArchetypeOrConcreteType.getAsConcreteType();
@@ -1248,16 +1239,6 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(
     return false;
   }
   
-  // Don't allow a generic parameter to be equivalent to a concrete type,
-  // because then we don't actually have a parameter.
-  // FIXME: Should we simply allow this?
-  if (T->getNestingDepth() == 0) {
-    Diags.diagnose(Source.getLoc(), 
-                   diag::requires_generic_param_made_equal_to_concrete,
-                   T->getName());
-    return true;
-  }
-  
   // Make sure the concrete type fulfills the requirements on the archetype.
   DenseMap<ProtocolDecl *, ProtocolConformance*> conformances;
   if (!Concrete->is<ArchetypeType>()) {
@@ -1752,8 +1733,67 @@ static Identifier typoCorrectNestedType(
   return bestMatches.front();
 }
 
-bool ArchetypeBuilder::finalize(SourceLoc loc) {
+bool
+ArchetypeBuilder::finalize(SourceLoc loc, bool allowConcreteGenericParams) {
   bool invalid = false;
+  SmallPtrSet<PotentialArchetype *, 4> visited;
+
+  // Check for generic parameters which have been made concrete or equated
+  // with each other.
+  if (!allowConcreteGenericParams) {
+    unsigned depth = 0;
+    for (const auto &pair : Impl->PotentialArchetypes) {
+      depth = std::max(depth, pair.second->getRootParam()->getDepth());
+    }
+
+    for (const auto &pair : Impl->PotentialArchetypes) {
+      auto pa = pair.second;
+      auto rep = pa->getRepresentative();
+
+      if (pa->getRootParam()->getDepth() < depth)
+        continue;
+
+      if (!visited.insert(rep).second)
+        continue;
+
+      // Don't allow a generic parameter to be equivalent to a concrete type,
+      // because then we don't actually have a parameter.
+      if (rep->ArchetypeOrConcreteType.getAsConcreteType()) {
+        auto &Source = rep->SameTypeSource;
+
+        // For auto-generated locations, we should have diagnosed the problem
+        // elsewhere already.
+        if (!Source->getLoc().isValid())
+          continue;
+
+        Diags.diagnose(Source->getLoc(),
+                       diag::requires_generic_param_made_equal_to_concrete,
+                       rep->getName());
+        invalid = true;
+        continue;
+      }
+
+      // Don't allow two generic parameters to be equivalent, because then we
+      // don't actually have two parameters.
+      for (auto other : rep->getEquivalenceClass()) {
+        if (pa != other && other->getParent() == nullptr) {
+          auto &Source = (other == rep ? pa->SameTypeSource
+                                       : other->SameTypeSource);
+
+          // For auto-generated locations, we should have diagnosed the problem
+          // elsewhere already.
+          if (!Source->getLoc().isValid())
+            continue;
+
+          Diags.diagnose(Source->getLoc(),
+                         diag::requires_generic_params_made_equal,
+                         pa->getName(), other->getName());
+          invalid = true;
+          break;
+        }
+      }
+    }
+  }
 
   // If any nested types remain unresolved, produce diagnostics.
   if (Impl->NumUnresolvedNestedTypes > 0) {

lib/Sema/TypeCheckDecl.cpp

@@ -730,6 +730,7 @@ TypeChecker::handleSILGenericParams(GenericParamList *genericParams,
     auto genericParams = nestedList.rbegin()[i];
     bool invalid = false;
     auto *genericSig = validateGenericSignature(genericParams, DC, parentSig,
+                                                /*allowConcreteGenericParams=*/true,
                                                 nullptr, invalid);
     if (invalid)
       return std::make_pair(nullptr, nullptr);
@@ -7545,11 +7546,10 @@ static Type checkExtensionGenericParams(
   bool invalid = false;
   auto *parentSig = ext->getDeclContext()->getGenericSignatureOfContext();
   auto *parentEnv = ext->getDeclContext()->getGenericEnvironmentOfContext();
-  GenericSignature *sig = tc.validateGenericSignature(
-      genericParams,
-      ext->getDeclContext(),
-      parentSig,
-      inferExtendedTypeReqs, invalid);
+  auto *sig = tc.validateGenericSignature(genericParams,
+                                          ext->getDeclContext(), parentSig,
+                                          /*allowConcreteGenericParams=*/false,
+                                          inferExtendedTypeReqs, invalid);
   ext->setGenericSignature(sig);
 
   if (invalid) {

lib/Sema/TypeCheckGeneric.cpp

@@ -725,6 +725,7 @@ GenericSignature *TypeChecker::validateGenericSignature(
                     GenericParamList *genericParams,
                     DeclContext *dc,
                     GenericSignature *parentSig,
+                    bool allowConcreteGenericParams,
                     std::function<bool(ArchetypeBuilder &)> inferRequirements,
                     bool &invalid) {
   assert(genericParams && "Missing generic parameters?");
@@ -747,7 +748,8 @@ GenericSignature *TypeChecker::validateGenericSignature(
   }
 
   // Finalize the generic requirements.
-  (void)builder.finalize(genericParams->getSourceRange().Start);
+  (void)builder.finalize(genericParams->getSourceRange().Start,
+                         allowConcreteGenericParams);
 
   // The archetype builder now has all of the requirements, although there might
   // still be errors that have not yet been diagnosed. Revert the signature
@@ -920,6 +922,7 @@ bool TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) {
   }
 
   auto *sig = validateGenericSignature(gp, dc, dc->getGenericSignatureOfContext(),
+                                       /*allowConcreteGenericParams=*/false,
                                        nullptr, invalid);
   assert(sig->getInnermostGenericParams().size()
            == typeDecl->getGenericParams()->size());

lib/Sema/TypeChecker.h

@@ -1056,6 +1056,7 @@ public:
                       GenericParamList *genericParams,
                       DeclContext *dc,
                       GenericSignature *outerSignature,
+                      bool allowConcreteGenericParams,
                       std::function<bool(ArchetypeBuilder &)> inferRequirements,
                       bool &invalid);
 

test/Constraints/same_types.swift

@@ -111,7 +111,7 @@ func testAssocTypeEquivalence<T: Fooable>(_ fooable: T) -> X.Type
 }
 
 func fail6<T>(_ t: T) -> Int where T == Int { // expected-error{{same-type requirement makes generic parameter 'T' non-generic}}
-  return t // expected-error{{cannot convert return expression of type 'T' to return type 'Int'}}
+  return t
 }
 
 func test8<T: Barrable, U: Barrable>(_ t: T, u: U) -> (Y, Y, X, X)
@@ -142,3 +142,66 @@ struct BadFooable : Fooable {
 func bogusInOutError(d: inout Brunch<BadFooable>) {}
 // expected-error@-1{{'Brunch' requires the types '<<error type>>' and 'X' be equivalent}}
 
+// Some interesting invalid cases that used to crash
+protocol P {
+  associatedtype A
+  associatedtype B
+}
+
+struct Q : P {
+  typealias A = Int
+  typealias B = Int
+}
+
+struct S1<T : P> {
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.B {
+    print(X.self)
+    print(Y.self)
+    print(x)
+    print(y)
+  }
+}
+S1<Q>().foo(x: 1, y: 2)
+
+struct S2<T : P> where T.A == T.B {
+  // expected-error@+1 {{same-type requirement makes generic parameters 'X' and 'Y' equivalent}}
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.B {
+    print(X.self)
+    print(Y.self)
+    print(x)
+    print(y)
+  }
+}
+S2<Q>().foo(x: 1, y: 2)
+
+struct S3<T : P> {
+  // expected-error@+1 {{same-type requirement makes generic parameters 'X' and 'Y' equivalent}}
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.A {}
+}
+S3<Q>().foo(x: 1, y: 2)
+
+// Secondaries can be equated OK, even if we're imposing
+// new conformances onto an outer secondary
+
+protocol PPP {}
+
+protocol PP {
+  associatedtype A : PPP
+}
+
+struct SSS : PPP {}
+struct SS : PP { typealias A = SSS }
+
+struct QQ : P {
+  typealias A = SSS
+  typealias B = Int
+}
+
+struct S4<T : P> {
+  func foo<X : PP>(x: X) where X.A == T.A {
+    print(x)
+    print(X.self)
+  }
+}
+
+S4<QQ>().foo(x: SS())

test/Generics/function_defs.swift

@@ -294,5 +294,5 @@ func badTypeConformance1<T>(_: T) where Int : EqualComparable {} // expected-err
 
 func badTypeConformance2<T>(_: T) where T.Blarg : EqualComparable { } // expected-error{{'Blarg' is not a member type of 'T'}}
 
-func badSameType<T, U : GeneratesAnElement, V>(_ : T) // expected-error{{generic parameter 'V' is not used in function signature}}
+func badSameType<T, U : GeneratesAnElement, V>(_ : T)
   where T == U.Element, U.Element == V {} // expected-error{{same-type requirement makes generic parameters 'T' and 'V' equivalent}}

test/SourceKit/CursorInfo/cursor_info.swift

@@ -421,32 +421,32 @@ func convention7(_: @convention(witness_method) ()->()) {}
 // CHECK33-NEXT: S2
 // CHECK33-NEXT: s:V11cursor_info2S2
 // CHECK33-NEXT: S2<T, U>.Type
-// CHECK33: <Declaration>struct S2&lt;T, U&gt;</Declaration>
-// CHECK33-NEXT: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>S2</decl.name>&lt;<decl.generic_type_param usr="s:tV11cursor_info2S21TMx"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt;</decl.struct>
+// CHECK33: <Declaration>struct S2&lt;T, U&gt; where T == U</Declaration>
+// CHECK33-NEXT: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>S2</decl.name>&lt;<decl.generic_type_param usr="s:tV11cursor_info2S21TMx"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="s:tV11cursor_info2S21UMq_"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt; <syntaxtype.keyword>where</syntaxtype.keyword> T == U</decl.struct>
 
 // RUN: %sourcekitd-test -req=cursor -pos=81:8 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %mcp_opt %s | %FileCheck %s -check-prefix=CHECK34
 // CHECK34: source.lang.swift.decl.function.method.instance (81:8-81:50)
 // CHECK34-NEXT: foo(_:)
-// CHECK34-NEXT: s:FV11cursor_info2S23foou0_rFFT_T_FT_T_
-// CHECK34-NEXT: <T, U, V, W> (S2<T, U>) -> (() -> ()) -> () -> ()
-// CHECK34: <Declaration>func foo&lt;V, W&gt;(_ closure: () -&gt; ()) -&gt; () -&gt; ()</Declaration>
-// CHECK34-NEXT: <decl.function.method.instance><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>foo</decl.name>&lt;<decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>V</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>W</decl.generic_type_param.name></decl.generic_type_param>&gt;(<decl.var.parameter><decl.var.parameter.argument_label>_</decl.var.parameter.argument_label> <decl.var.parameter.name>closure</decl.var.parameter.name>: <decl.var.parameter.type>() -&gt; <decl.function.returntype><tuple>()</tuple></decl.function.returntype></decl.var.parameter.type></decl.var.parameter>) -&gt; <decl.function.returntype>() -&gt; <decl.function.returntype><tuple>()</tuple></decl.function.returntype></decl.function.returntype></decl.function.method.instance>
+// CHECK34-NEXT: s:FV11cursor_info2S23foou0_Rd__zqd_0_rFFT_T_FT_T_
+// CHECK34-NEXT: <T, U, V, W where T == U, V == W> (S2<T, U>) -> (() -> ()) -> () -> ()
+// CHECK34: <Declaration>func foo&lt;V, W&gt;(_ closure: () -&gt; ()) -&gt; () -&gt; () where V == W</Declaration>
+// CHECK34-NEXT: <decl.function.method.instance><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>foo</decl.name>&lt;<decl.generic_type_param usr="s:tFV11cursor_info2S23foou0_Rd__zqd_0_rFFT_T_FT_T_L_1VMqd__"><decl.generic_type_param.name>V</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="s:tFV11cursor_info2S23foou0_Rd__zqd_0_rFFT_T_FT_T_L_1WMqd_0_"><decl.generic_type_param.name>W</decl.generic_type_param.name></decl.generic_type_param>&gt;(<decl.var.parameter><decl.var.parameter.argument_label>_</decl.var.parameter.argument_label> <decl.var.parameter.name>closure</decl.var.parameter.name>: <decl.var.parameter.type>() -&gt; <decl.function.returntype><tuple>()</tuple></decl.function.returntype></decl.var.parameter.type></decl.var.parameter>) -&gt; <decl.function.returntype>() -&gt; <decl.function.returntype><tuple>()</tuple></decl.function.returntype></decl.function.returntype> <syntaxtype.keyword>where</syntaxtype.keyword> V == W</decl.function.method.instance>
 
 // RUN: %sourcekitd-test -req=cursor -pos=83:7 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %mcp_opt %s | %FileCheck %s -check-prefix=CHECK35
 // CHECK35: source.lang.swift.decl.class (83:7-83:9)
 // CHECK35-NEXT: C4
 // CHECK35-NEXT: s:C11cursor_info2C4
 // CHECK35-NEXT: C4<T, U>.Type
-// CHECK35: <Declaration>class C4&lt;T, U&gt;</Declaration>
-// CHECK35-NEXT: <decl.class><syntaxtype.keyword>class</syntaxtype.keyword> <decl.name>C4</decl.name>&lt;<decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt;</decl.class>
+// CHECK35: <Declaration>class C4&lt;T, U&gt; where T == U</Declaration>
+// CHECK35-NEXT: <decl.class><syntaxtype.keyword>class</syntaxtype.keyword> <decl.name>C4</decl.name>&lt;<decl.generic_type_param usr="s:tC11cursor_info2C41TMx"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="s:tC11cursor_info2C41UMq_"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt; <syntaxtype.keyword>where</syntaxtype.keyword> T == U</decl.class>
 
 // RUN: %sourcekitd-test -req=cursor -pos=84:6 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %mcp_opt %s | %FileCheck %s -check-prefix=CHECK36
 // CHECK36: source.lang.swift.decl.enum (84:6-84:8)
 // CHECK36-NEXT: E1
 // CHECK36-NEXT: s:O11cursor_info2E1
 // CHECK36-NEXT: E1<T, U>.Type
-// CHECK36: <Declaration>enum E1&lt;T, U&gt;</Declaration>
-// CHECK36-NEXT: <decl.enum><syntaxtype.keyword>enum</syntaxtype.keyword> <decl.name>E1</decl.name>&lt;<decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="{{.*}}"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt;</decl.enum>
+// CHECK36: <Declaration>enum E1&lt;T, U&gt; where T == U</Declaration>
+// CHECK36-NEXT: <decl.enum><syntaxtype.keyword>enum</syntaxtype.keyword> <decl.name>E1</decl.name>&lt;<decl.generic_type_param usr="s:tO11cursor_info2E11TMx"><decl.generic_type_param.name>T</decl.generic_type_param.name></decl.generic_type_param>, <decl.generic_type_param usr="s:tO11cursor_info2E11UMq_"><decl.generic_type_param.name>U</decl.generic_type_param.name></decl.generic_type_param>&gt; <syntaxtype.keyword>where</syntaxtype.keyword> T == U</decl.enum>
 
 // RUN: %sourcekitd-test -req=cursor -pos=86:6 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %mcp_opt %s | %FileCheck %s -check-prefix=CHECK37
 // CHECK37: source.lang.swift.decl.function.free (86:6-86:111)

test/decl/protocol/req/recursion.swift

@@ -7,8 +7,11 @@ protocol SomeProtocol {
 extension SomeProtocol where T == Optional<T> { } // expected-error{{same-type constraint 'Self.T' == 'Optional<Self.T>' is recursive}}
 
 // rdar://problem/19840527
-class X<T> where T == X { // expected-error{{same-type requirement makes generic parameter 'T' non-generic}}
-    var type: T { return type(of: self) } // expected-error{{cannot convert return expression of type 'X<T>.Type' to return type 'T'}}
+// FIXME: Crappy diagnostic
+
+class X<T> where T == X { // expected-error{{non-class type '<<error type>>' cannot conform to class protocol 'AnyObject'}}
+// expected-error@-1{{same-type requirement makes generic parameter 'T' non-generic}}
+    var type: T { return type(of: self) } // expected-error{{use of undeclared type 'T'}}
 }
 
 protocol Y {
@@ -23,7 +26,7 @@ public protocol P {
 public struct S<A: P> where A.T == S<A> {} // expected-error{{type may not reference itself as a requirement}}
 
 protocol I {
-  init() // expected-note{{protocol requires initializer 'init()' with type '()'}}
+  init() // expected-note 2{{protocol requires initializer 'init()' with type '()'}}
 }
 
 protocol PI {
@@ -33,18 +36,16 @@ protocol PI {
 struct SI<A: PI> : I where A : I, A.T == SI<A> { // expected-error{{type may not reference itself as a requirement}}
 }
 
-/* FIXME: Infinite recursion
-
+// Used to hit infinite recursion
 struct S4<A: PI> : I where A : I {
 }
 
 struct S5<A: PI> : I where A : I, A.T == S4<A> {
-}*/
-
-/* FIXME: Hits ArchetypeBuilder assertions
+}
 
+// Used to hit ArchetypeBuilder assertions
 struct SU<A: P> where A.T == SU {
 }
 
 struct SIU<A: PI> : I where A : I, A.T == SIU {
-}*/
+}