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

In Swift 4, we only gave custom overload types to properties defined in extensions of generic types, using the null type for any other var decl. This meant that a property defined in such an extension would never shadow a property not defined in such an extension. As a result, this permitted cross-module overloads of properties of different types on generic types in certain cases.

This patch adds an exception to the shadowing rules for properties defined in generic type extensions under Swift 4 mode. Permitting cross-module property overloads in the general case would also be source breaking (causing ambiguity errors), so this can be handled in a follow-up Swift 5 mode PR if desired.

Resolves SR-7341.

Author: hamishknight

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()

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
+  }
+}