Merge pull request swiftlang#21502 from DougGregor/dynamic-replacement-ambiguity

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -3886,6 +3886,10 @@ ERROR(dynamic_replacement_function_not_found, none,
      "replaced function %0 could not be found", (DeclName))
 ERROR(dynamic_replacement_accessor_not_found, none,
       "replaced accessor for %0 could not be found", (DeclName))
+ERROR(dynamic_replacement_accessor_ambiguous, none,
+      "replaced accessor for %0 occurs in multiple places", (DeclName))
+NOTE(dynamic_replacement_accessor_ambiguous_candidate, none,
+      "candidate accessor found in module %0", (DeclName))
 ERROR(dynamic_replacement_function_of_type_not_found, none,
       "replaced function %0 of type %1 could not be found", (DeclName, Type))
 NOTE(dynamic_replacement_found_function_of_type, none,

lib/Sema/TypeCheckAttr.cpp

@@ -2036,8 +2036,9 @@ void lookupReplacedDecl(DeclName replacedDeclName,
                              replacement->getModuleScopeContext(), nullptr,
                              attr->getLocation());
     if (lookup.isSuccess()) {
-      for (auto entry : lookup.Results)
+      for (auto entry : lookup.Results) {
         results.push_back(entry.getValueDecl());
+      }
     }
     return;
   }
@@ -2051,6 +2052,28 @@ void lookupReplacedDecl(DeclName replacedDeclName,
       {typeCtx}, replacedDeclName, NL_QualifiedDefault, results);
 }
 
+/// Remove any argument labels from the interface type of the given value that
+/// are extraneous from the type system's point of view, producing the
+/// type to compare against for the purposes of dynamic replacement.
+static Type getDynamicComparisonType(ValueDecl *value) {
+  unsigned numArgumentLabels = 0;
+
+  if (isa<AbstractFunctionDecl>(value)) {
+    ++numArgumentLabels;
+
+    if (value->getDeclContext()->isTypeContext())
+      ++numArgumentLabels;
+  } else if (isa<SubscriptDecl>(value)) {
+    ++numArgumentLabels;
+  }
+
+  auto interfaceType = value->getInterfaceType();
+  if (interfaceType->hasError())
+    return interfaceType;
+
+  return interfaceType->removeArgumentLabels(numArgumentLabels);
+}
+
 static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
                                       AccessorDecl *replacement,
                                       DynamicReplacementAttr *attr,
@@ -2060,13 +2083,47 @@ static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
   SmallVector<ValueDecl *, 4> results;
   lookupReplacedDecl(replacedVarName, attr, replacement, results);
 
+  // Filter out any accessors that won't work.
+  if (!results.empty()) {
+    auto replacementStorage = replacement->getStorage();
+    TC.validateDecl(replacementStorage);
+    Type replacementStorageType = getDynamicComparisonType(replacementStorage);
+    results.erase(std::remove_if(results.begin(), results.end(),
+        [&](ValueDecl *result) {
+          // Check for static/instance mismatch.
+          if (result->isStatic() != replacementStorage->isStatic())
+            return true;
+
+          // Check for type mismatch.
+          TC.validateDecl(result);
+          auto resultType = getDynamicComparisonType(result);
+          if (!resultType->isEqual(replacementStorageType)) {
+            return true;
+          }
+
+          return false;
+        }),
+        results.end());
+  }
+
   if (results.empty()) {
     TC.diagnose(attr->getLocation(),
                 diag::dynamic_replacement_accessor_not_found, replacedVarName);
     attr->setInvalid();
     return nullptr;
   }
-  assert(results.size() == 1 && "Should only have on var or fun");
+
+  if (results.size() > 1) {
+    TC.diagnose(attr->getLocation(),
+                diag::dynamic_replacement_accessor_ambiguous, replacedVarName);
+    for (auto result : results) {
+      TC.diagnose(result,
+                  diag::dynamic_replacement_accessor_ambiguous_candidate,
+                  result->getModuleContext()->getFullName());
+    }
+    attr->setInvalid();
+    return nullptr;
+  }
 
   assert(!isa<FuncDecl>(results[0]));
   TC.validateDecl(results[0]);
@@ -2117,6 +2174,10 @@ findReplacedFunction(DeclName replacedFunctionName,
   lookupReplacedDecl(replacedFunctionName, attr, replacement, results);
 
   for (auto *result : results) {
+    // Check for static/instance mismatch.
+    if (result->isStatic() != replacement->isStatic())
+      continue;
+
     TC.validateDecl(result);
     if (result->getInterfaceType()->getCanonicalType()->matches(
             replacement->getInterfaceType()->getCanonicalType(),

test/attr/Inputs/dynamicReplacementA.swift

@@ -0,0 +1,2 @@
+public struct TheReplaceables {
+}

test/attr/Inputs/dynamicReplacementB.swift

@@ -0,0 +1,9 @@
+import A
+
+public extension TheReplaceables {
+  dynamic var property1: Int { return 0 }
+  dynamic var property2: Int { return 0 }
+
+  dynamic subscript (i: Int) -> Int { return 0 }
+  dynamic subscript (i: Int) -> String { return "" }
+}

test/attr/Inputs/dynamicReplacementC.swift

@@ -0,0 +1,9 @@
+import A
+
+public extension TheReplaceables {
+  dynamic var property1: Int { return 0 }
+  dynamic var property2: String { return "" }
+
+  dynamic subscript (i: Int) -> Int { return 0 }
+  dynamic subscript (s: String) -> String { return "" }
+}

test/attr/dynamicReplacement.swift

@@ -0,0 +1,40 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic %S/Inputs/dynamicReplacementA.swift -o %t -module-name A
+// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic -c %S/Inputs/dynamicReplacementB.swift -o %t -I %t -module-name B
+// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic -c %S/Inputs/dynamicReplacementC.swift -o %t -I %t -module-name C
+// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-implicit-dynamic  -I %t
+import A
+import B
+import C
+
+// rdar://problem/46737657: static properties
+struct StaticProperties {
+  dynamic var property: Int { return 1 }
+  dynamic static var property: Int { return 11 }
+}
+
+extension StaticProperties {
+  @_dynamicReplacement(for: property)
+  var replacement_property: Int { return 2 }
+}
+
+// Replacements involving different types.
+extension TheReplaceables {
+  @_dynamicReplacement(for: property1) // expected-error{{replaced accessor for 'property1' occurs in multiple places}}
+  var replace_property1: Int { return 0 }
+  
+  @_dynamicReplacement(for: property2)
+  var replace_property2_int: Int { return 1 }
+  
+  @_dynamicReplacement(for: property2)
+  var replace_property2_string: String { return "replaced" }
+
+  @_dynamicReplacement(for: subscript(_:)) // expected-error{{replaced accessor for 'subscript(_:)' occurs in multiple places}}
+  subscript (int_int i: Int) -> Int { return 0 }
+
+  @_dynamicReplacement(for: subscript(_:))
+  subscript (int_string i: Int) -> String { return "" }
+
+  @_dynamicReplacement(for: subscript(_:))
+  subscript (string_string i: String) -> String { return "" }
+}