Sema: Tighten ridiculous disambiguation hack for overloaded type declarations

We noticed that a project failed to build with prepared overloads enabled,
but built successfully with prepared overloads disabled.

It turns out that the code clearly should never have been accepted in the
first place, because the type checker was making an arbitrary choice between
two nominal type declarations with the same name.

Further inspection revealed this was because of a FIXME added in 2013 which
was far too broad. Tighten up the logic here to only disambiguate if at least
one of the two declarations is a type alias, and it has the same underlying
type as the other one. A choice between unrelated nominal type declarations
should always be ambiguous.

This still isn't perfect, because we might have two generic type aliases
that are referenced in both solutions with different substitution maps,
in which case we will still erroneously pick the first one. But this
is better than the old logic, at least.

Fixes rdar://165863775.

Author: slavapestov

lib/Sema/CSRanking.cpp

@@ -195,14 +195,19 @@ static bool sameDecl(Decl *decl1, Decl *decl2) {
   if (decl1 == decl2)
     return true;
 
-  // All types considered identical.
-  // FIXME: This is a hack. What we really want is to have substituted the
-  // base type into the declaration reference, so that we can compare the
-  // actual types to which two type declarations resolve. If those types are
-  // equivalent, then it doesn't matter which declaration is chosen.
-  if (isa<TypeDecl>(decl1) && isa<TypeDecl>(decl2))
-    return true;
-  
+  // Special hack to allow type aliases with same underlying type.
+  //
+  // FIXME: Check substituted types instead.
+  //
+  // FIXME: Perhaps this should be handled earlier in name lookup.
+  auto *typeDecl1 = dyn_cast<TypeDecl>(decl1);
+  auto *typeDecl2 = dyn_cast<TypeDecl>(decl2);
+  if (typeDecl1 && typeDecl2) {
+    auto type1 = typeDecl1->getDeclaredInterfaceType();
+    auto type2 = typeDecl2->getDeclaredInterfaceType();
+    return type1->isEqual(type2);
+  }
+
   if (decl1->getKind() != decl2->getKind())
     return false;
 

test/AssociatedTypeInference/type_witness_from_parameterized_protocol.swift

@@ -43,10 +43,12 @@ struct S4: (P & Q<String>) & R {
 }
 
 struct Bad: P<Int, Float> { // expected-error {{type 'Bad' does not conform to protocol 'P'}}
-  typealias A = String  // expected-note {{possibly intended match}}
+  typealias A = String
+  // expected-note@-1 {{possibly intended match}}
+  // expected-note@-2 {{found this candidate}}
 }
 
-let x = Bad.A.self
+let x = Bad.A.self // expected-error {{ambiguous use of 'A'}}
 g(x)
 
 struct Circle: Q<Circle.A> {}

test/Constraints/ambiguous_specialized_name_diagnostics.swift

@@ -38,10 +38,10 @@ import A
 import B
 
 func test() {
-  _ = S<Int>(t: 42) // expected-error {{ambiguous use of 'init(t:)'}}
+  _ = S<Int>(t: 42) // expected-error {{ambiguous use of 'S'}}
 
-  S<Int>(t: 42).test() // expected-error {{ambiguous use of 'init(t:)'}}
+  S<Int>(t: 42).test() // expected-error {{ambiguous use of 'S'}}
 
   S<Int>.staticFn()
-  // expected-error@-1 {{ambiguous use of 'staticFn()'}}
+  // expected-error@-1 {{ambiguous use of 'S'}}
 }

test/Constraints/invalid_decl_ref.swift

@@ -1,9 +1,7 @@
-// RUN: %target-swift-frontend -module-name SomeModule -typecheck -verify -dump-ast -import-objc-header %S/Inputs/imported_type.h %s | %FileCheck %s
+// RUN: %target-swift-frontend -module-name SomeModule -typecheck -verify -import-objc-header %S/Inputs/imported_type.h %s -verify-ignore-unrelated
 
 // REQUIRES: objc_interop
 
 import Foundation
 
-// CHECK: declref_expr type="module<SomeModule>"
-// CHECK-NEXT: type_expr type="Data.Type"
-let type = SomeModule.Data.self
+let type = SomeModule.Data.self // expected-error {{ambiguous use of 'Data'}}

test/Constraints/rdar165863775.swift

@@ -0,0 +1,40 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/src)
+// RUN: split-file %s %t/src
+
+// RUN: %target-swift-frontend -emit-module %t/src/A.swift \
+// RUN:   -module-name A \
+// RUN:   -emit-module-path %t/A.swiftmodule
+
+// RUN: %target-swift-frontend -emit-module %t/src/B.swift \
+// RUN:   -module-name B \
+// RUN:   -emit-module-path %t/B.swiftmodule
+
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -module-name main -I %t %t/src/main.swift
+
+//--- A.swift
+public struct Bag {
+}
+
+//--- B.swift
+public struct Bag {
+}
+
+//--- main.swift
+import A
+import B
+
+protocol P {
+}
+
+struct Test {
+    func inject<T>(_: T.Type = T.self) {}
+
+    func inject<T: P>(_: T) {}
+
+    func inject<T>(_: T) async -> T {}
+}
+
+func test(t: Test) {
+    t.inject(Bag.self)  // expected-error {{ambiguous use of 'Bag'}}
+}

test/NameLookup/member_import_visibility.swift

@@ -85,7 +85,7 @@ extension X {
     _ = (NestedInA, NestedInB, NestedInC).self // expected-member-visibility-error{{struct 'NestedInB' is not available due to missing import of defining module 'members_B'}}
     _ = GenericType<NestedInB>.self // expected-member-visibility-error{{struct 'NestedInB' is not available due to missing import of defining module 'members_B'}}
     _ = NestedInC.self
-    _ = AmbiguousNestedType.self
+    _ = AmbiguousNestedType.self // expected-ambiguity-error{{ambiguous use of 'AmbiguousNestedType'}}
   }
 
   var hasNestedInAType: NestedInA { fatalError() }

validation-test/Sema/rdar84879566.swift

@@ -20,6 +20,6 @@ struct TupleBuilder {
 
 struct MyApp: Tupled {
   var tuple: some Any {
-    MyView() // expected-error {{ambiguous use of 'init()'}}
+    MyView() // expected-error {{ambiguous use of 'MyView'}}
   }
 }

validation-test/compiler_crashers_fixed/issue-85364.swift

@@ -9,7 +9,7 @@ struct S1 {  // expected-note {{found this candidate}} \
   var c: () -> Void
 }
 
-S1 {}  // expected-error {{ambiguous use of 'init'}}
+S1 {}  // expected-error {{ambiguous use of 'S1'}}
 
 struct S1 {  // expected-note {{found this candidate}} \
              // expected-error {{invalid redeclaration of 'S1'}}