Merge pull request #18488 from hamishknight/swift4-cross-module-var-overload

[NameLookup] Swift 4 compatibility hack for cross-module var overloads in generic type extensions

Author: slavapestov

lib/AST/NameLookup.cpp

@@ -172,11 +172,25 @@ static void recordShadowedDeclsAfterSignatureMatch(
   for (unsigned firstIdx : indices(decls)) {
     auto firstDecl = decls[firstIdx];
     auto firstModule = firstDecl->getModuleContext();
+    auto firstSig = firstDecl->getOverloadSignature();
     for (unsigned secondIdx : range(firstIdx + 1, decls.size())) {
       // Determine whether one module takes precedence over another.
       auto secondDecl = decls[secondIdx];
       auto secondModule = secondDecl->getModuleContext();
 
+      // Swift 4 compatibility hack: Don't shadow properties defined in
+      // extensions of generic types with properties defined elsewhere.
+      // This is due to the fact that in Swift 4, we only gave custom overload
+      // types to properties in extensions of generic types, otherwise we
+      // used the null type.
+      if (!ctx.isSwiftVersionAtLeast(5)) {
+        auto secondSig = secondDecl->getOverloadSignature();
+        if (firstSig.IsVariable && secondSig.IsVariable)
+          if (firstSig.InExtensionOfGenericType !=
+              secondSig.InExtensionOfGenericType)
+            continue;
+      }
+
       // If one declaration is in a protocol or extension thereof and the
       // other is not, prefer the one that is not.
       if ((bool)firstDecl->getDeclContext()->getSelfProtocolDecl() !=

test/Constraints/dynamic_lookup.swift

@@ -340,3 +340,19 @@ func dynamicInitCrash(ao: AnyObject.Type) {
   let sdk = ao.init(blahblah: ())
   // expected-error@-1 {{incorrect argument label in call (have 'blahblah:', expected 'toMemory:')}}
 }
+
+// Test that we correctly diagnose ambiguity for different typed properties available
+// through dynamic lookup.
+@objc protocol P3 {
+  var i: UInt { get } // expected-note {{found this candidate}}
+  var j: Int { get }
+}
+
+class C1 {
+  @objc var i: Int { return 0 } // expected-note {{found this candidate}}
+  @objc var j: Int { return 0 }
+}
+
+_ = (C1() as AnyObject).i // expected-error {{ambiguous use of 'i'}}
+_ = (C1() as AnyObject).j // okay
+

test/IDE/Inputs/complete_import_overloads.swift

@@ -0,0 +1,16 @@
+
+public struct HasFooGeneric<T> {
+  public var foo: Int = 0
+}
+
+extension HasFooGeneric {
+  public var bar: Int { return 0 }
+}
+
+public class HasFooNonGeneric {
+  public var foo: Int = 0
+}
+
+extension HasFooNonGeneric {
+  public var bar: Int { return 0 }
+}

test/IDE/complete_import_overloads.swift

@@ -0,0 +1,66 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -emit-module -o %t -module-name=library %S/Inputs/complete_import_overloads.swift
+
+// RUN: %target-swift-ide-test -code-completion -swift-version 4 -source-filename %s -code-completion-token=SELF_DOT_1 -I %t | %FileCheck %s -check-prefix=SWIFT4_SELF_DOT_1
+// RUN: %target-swift-ide-test -code-completion -swift-version 4 -source-filename %s -code-completion-token=SELF_DOT_2 -I %t | %FileCheck %s -check-prefix=SWIFT4_SELF_DOT_2
+
+// RUN: %target-swift-ide-test -code-completion -swift-version 5 -source-filename %s -code-completion-token=SELF_DOT_1 -I %t | %FileCheck %s -check-prefix=SWIFT5_SELF_DOT_1
+// RUN: %target-swift-ide-test -code-completion -swift-version 5 -source-filename %s -code-completion-token=SELF_DOT_2 -I %t | %FileCheck %s -check-prefix=SWIFT5_SELF_DOT_2
+
+import library
+
+// Ensure we maintain compatibility with Swift 4's overload signature rules.
+// Variables defined in extensions of generic types had different overload
+// signatures to other variables, so allow overloading in such cases (SR-7341).
+extension HasFooGeneric {
+  var foo: String { return "" } // foo isn't defined in a generic extension in the other module, so allow overloading in Swift 4 mode.
+  var bar: String { return "" } // bar is defined in a generic extension in the other module, so `bar: String` always shadows it.
+  func baz() {
+    self.#^SELF_DOT_1^#
+  }
+}
+
+// SWIFT4_SELF_DOT_1: Begin completions
+// SWIFT4_SELF_DOT_1-DAG: Decl[InstanceVar]/CurrNominal:      foo[#Int#]; name=foo
+// SWIFT4_SELF_DOT_1-DAG: Decl[InstanceVar]/CurrNominal:      foo[#String#]; name=foo
+// SWIFT4_SELF_DOT_1-NOT: Decl[InstanceVar]/CurrNominal:      bar[#Int#]; name=bar
+// SWIFT4_SELF_DOT_1-DAG: Decl[InstanceVar]/CurrNominal:      bar[#String#]; name=bar
+// SWIFT4_SELF_DOT_1: End completions
+
+// But in Swift 5 mode, properties from this module currently always shadow
+// properties from the other module – therefore meaning that the properties from
+// the other module never show up in the overload set.
+// FIX-ME: It seems reasonable for both to show up in the overload set.
+// SWIFT5_SELF_DOT_1: Begin completions
+// SWIFT5_SELF_DOT_1-NOT: Decl[InstanceVar]/CurrNominal:      foo[#Int#]; name=foo
+// SWIFT5_SELF_DOT_1-DAG: Decl[InstanceVar]/CurrNominal:      foo[#String#]; name=foo
+// SWIFT5_SELF_DOT_1-NOT: Decl[InstanceVar]/CurrNominal:      bar[#Int#]; name=bar
+// SWIFT5_SELF_DOT_1-DAG: Decl[InstanceVar]/CurrNominal:      bar[#String#]; name=bar
+// SWIFT5_SELF_DOT_1: End completions
+
+// For non-generic types, the variable overload signature was always the
+// null type, so `foo/bar: String` shadows `foo/bar: Int`.
+extension HasFooNonGeneric {
+  var foo: String { return "" }
+  var bar: String { return "" }
+  func baz() {
+    self.#^SELF_DOT_2^#
+  }
+}
+
+// SWIFT4_SELF_DOT_2: Begin completions
+// SWIFT4_SELF_DOT_2-NOT: Decl[InstanceVar]/CurrNominal:      foo[#Int#]; name=foo
+// SWIFT4_SELF_DOT_2-DAG: Decl[InstanceVar]/CurrNominal:      foo[#String#]; name=foo
+// SWIFT4_SELF_DOT_2-NOT: Decl[InstanceVar]/CurrNominal:      bar[#Int#]; name=bar
+// SWIFT4_SELF_DOT_2-DAG: Decl[InstanceVar]/CurrNominal:      bar[#String#]; name=bar
+// SWIFT4_SELF_DOT_2: End completions
+
+// Again, in Swift 5 mode, we currently consistently shadow the properties from
+// the other module.
+// FIX-ME: It seems reasonable to not shadow them.
+// SWIFT5_SELF_DOT_2: Begin completions
+// SWIFT5_SELF_DOT_2-NOT: Decl[InstanceVar]/CurrNominal:      foo[#Int#]; name=foo
+// SWIFT5_SELF_DOT_2-DAG: Decl[InstanceVar]/CurrNominal:      foo[#String#]; name=foo
+// SWIFT5_SELF_DOT_2-NOT: Decl[InstanceVar]/CurrNominal:      bar[#Int#]; name=bar
+// SWIFT5_SELF_DOT_2-DAG: Decl[InstanceVar]/CurrNominal:      bar[#String#]; name=bar
+// SWIFT5_SELF_DOT_2: End completions

test/NameBinding/Inputs/overload_vars.swift

@@ -14,3 +14,19 @@ public protocol HasFoo {
 public protocol HasBar {
   var bar: Int { get }
 }
+
+public class HasFooGeneric<T> {
+  public var foo: Int = 0
+}
+
+extension HasFooGeneric {
+  public var bar: Int { return 0 }
+}
+
+public class HasFooNonGeneric {
+  public var foo: Int = 0
+}
+
+extension HasFooNonGeneric {
+  public var bar: Int { return 0 }
+}

test/NameBinding/import-resolution-overload_swift4.swift

@@ -0,0 +1,53 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/overload_vars.swift
+// RUN: %target-typecheck-verify-swift -swift-version 4 -I %t
+
+import overload_vars
+
+func useString(_ str: String) {}
+
+// Ensure we maintain compatibility with Swift 4's overload signature rules.
+// Variables defined in extensions of generic types had different overload
+// signatures to other variables, so allow overloading in such cases (SR-7341).
+extension HasFooGeneric {
+  var foo: String { return "" } // `foo` isn't defined in a generic extension in the other module, so allow overloading in Swift 4 mode.
+  var bar: String { return "" } // `bar` is defined in a generic extension in the other module, so `bar: String` always shadows it.
+
+  func baz() {
+    let x1: Int = foo // Make sure `foo: Int` is in the overload set.
+    _ = x1
+
+    let x2: String = foo // Make sure `foo: String` is in the overload set.
+    _ = x2
+
+    let y1 = bar // No ambiguity error.
+    useString(y1) // Make sure we resolved to `bar: String`.
+
+    // Make sure `bar: Int` is not in the overload set.
+    let y2: Int = bar // expected-error {{cannot convert}}
+    _ = y2
+  }
+}
+
+// But for non-generic types, the variable overload signature was always the
+// null type, so `foo/bar: String` shadows `foo/bar: Int`.
+extension HasFooNonGeneric {
+  var foo: String { return "" }
+  var bar: String { return "" }
+
+  func baz() {
+    let x1 = foo // No ambiguity error.
+    useString(x1) // Make sure we resolved to `foo: String`.
+
+    // Make sure `foo: Int` is not in the overload set.
+    let x2: Int = foo // expected-error {{cannot convert}}
+    _ = x2
+
+    let y1 = bar // No ambiguity error.
+    useString(y1) // Make sure we resolved to `bar: String`.
+
+    // Make sure `bar: Int` is not in the overload set.
+    let y2: Int = bar // expected-error {{cannot convert}}
+    _ = y2
+  }
+}

test/NameBinding/import-resolution-overload_swift5.swift

@@ -0,0 +1,54 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/overload_vars.swift
+// RUN: %target-typecheck-verify-swift -swift-version 5 -I %t
+
+import overload_vars
+
+func useString(_ str: String) {}
+
+// In Swift 5, properties from this module currently always shadow properties
+// from the other module – therefore meaning that the properties from the other
+// module never show up in the overload set.
+// FIX-ME: It seems reasonable for both to show up in the overload set.
+
+extension HasFooGeneric {
+  var foo: String { return "" }
+  var bar: String { return "" }
+
+  func baz() {
+    let x1 = foo // No ambiguity error.
+    useString(x1) // Make sure we resolved to `foo: String`.
+
+    // Make sure `foo: Int` is not in the overload set.
+    let x2: Int = foo // expected-error {{cannot convert}}
+    _ = x2
+
+    let y1 = bar // No ambiguity error.
+    useString(y1) // Make sure we resolved to `bar: String`.
+
+    // Make sure `bar: Int` is not in the overload set.
+    let y2: Int = bar // expected-error {{cannot convert}}
+    _ = y2
+  }
+}
+
+extension HasFooNonGeneric {
+  var foo: String { return "" }
+  var bar: String { return "" }
+
+  func baz() {
+    let x1 = foo // No ambiguity error.
+    useString(x1) // Make sure we resolved to `foo: String`.
+
+    // Make sure `foo: Int` is not in the overload set.
+    let x2: Int = foo // expected-error {{cannot convert}}
+    _ = x2
+
+    let y1 = bar // No ambiguity error.
+    useString(y1) // Make sure we resolved to `bar: String`.
+
+    // Make sure `bar: Int` is not in the overload set.
+    let y2: Int = bar // expected-error {{cannot convert}}
+    _ = y2
+  }
+}