Skip to content

Commit 5d96862

Browse files
committed
[Type checker] Basic ambiguity resolution + diagnostics for dynamic replacement.
We weren't doing much validation when dynamically replacing storage declarations, and has an assert() that should be an error. Clean up this area a bit, dealing with simple ambiguities (i.e., there are two properties or subscripts with different type signatures; pick the matching one) and reporting an error when there is a true ambiguity. Fixes rdar://problem/46737657.
1 parent f223d0b commit 5d96862

File tree

6 files changed

+127
-2
lines changed

6 files changed

+127
-2
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3886,6 +3886,10 @@ ERROR(dynamic_replacement_function_not_found, none,
38863886
"replaced function %0 could not be found", (DeclName))
38873887
ERROR(dynamic_replacement_accessor_not_found, none,
38883888
"replaced accessor for %0 could not be found", (DeclName))
3889+
ERROR(dynamic_replacement_accessor_ambiguous, none,
3890+
"replaced accessor for %0 occurs in multiple places", (DeclName))
3891+
NOTE(dynamic_replacement_accessor_ambiguous_candidate, none,
3892+
"candidate accessor found in module %0", (DeclName))
38893893
ERROR(dynamic_replacement_function_of_type_not_found, none,
38903894
"replaced function %0 of type %1 could not be found", (DeclName, Type))
38913895
NOTE(dynamic_replacement_found_function_of_type, none,

lib/Sema/TypeCheckAttr.cpp

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,8 +2036,9 @@ void lookupReplacedDecl(DeclName replacedDeclName,
20362036
replacement->getModuleScopeContext(), nullptr,
20372037
attr->getLocation());
20382038
if (lookup.isSuccess()) {
2039-
for (auto entry : lookup.Results)
2039+
for (auto entry : lookup.Results) {
20402040
results.push_back(entry.getValueDecl());
2041+
}
20412042
}
20422043
return;
20432044
}
@@ -2051,6 +2052,28 @@ void lookupReplacedDecl(DeclName replacedDeclName,
20512052
{typeCtx}, replacedDeclName, NL_QualifiedDefault, results);
20522053
}
20532054

2055+
/// Remove any argument labels from the interface type of the given value that
2056+
/// are extraneous from the type system's point of view, producing the
2057+
/// type to compare against for the purposes of dynamic replacement.
2058+
static Type getDynamicComparisonType(ValueDecl *value) {
2059+
unsigned numArgumentLabels = 0;
2060+
2061+
if (isa<AbstractFunctionDecl>(value)) {
2062+
++numArgumentLabels;
2063+
2064+
if (value->getDeclContext()->isTypeContext())
2065+
++numArgumentLabels;
2066+
} else if (isa<SubscriptDecl>(value)) {
2067+
++numArgumentLabels;
2068+
}
2069+
2070+
auto interfaceType = value->getInterfaceType();
2071+
if (interfaceType->hasError())
2072+
return interfaceType;
2073+
2074+
return interfaceType->removeArgumentLabels(numArgumentLabels);
2075+
}
2076+
20542077
static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
20552078
AccessorDecl *replacement,
20562079
DynamicReplacementAttr *attr,
@@ -2060,13 +2083,47 @@ static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
20602083
SmallVector<ValueDecl *, 4> results;
20612084
lookupReplacedDecl(replacedVarName, attr, replacement, results);
20622085

2086+
// Filter out any accessors that won't work.
2087+
if (!results.empty()) {
2088+
auto replacementStorage = replacement->getStorage();
2089+
TC.validateDecl(replacementStorage);
2090+
Type replacementStorageType = getDynamicComparisonType(replacementStorage);
2091+
results.erase(std::remove_if(results.begin(), results.end(),
2092+
[&](ValueDecl *result) {
2093+
// Check for static/instance mismatch.
2094+
if (result->isStatic() != replacementStorage->isStatic())
2095+
return true;
2096+
2097+
// Check for type mismatch.
2098+
TC.validateDecl(result);
2099+
auto resultType = getDynamicComparisonType(result);
2100+
if (!resultType->isEqual(replacementStorageType)) {
2101+
return true;
2102+
}
2103+
2104+
return false;
2105+
}),
2106+
results.end());
2107+
}
2108+
20632109
if (results.empty()) {
20642110
TC.diagnose(attr->getLocation(),
20652111
diag::dynamic_replacement_accessor_not_found, replacedVarName);
20662112
attr->setInvalid();
20672113
return nullptr;
20682114
}
2069-
assert(results.size() == 1 && "Should only have on var or fun");
2115+
2116+
if (results.size() > 1) {
2117+
TC.diagnose(attr->getLocation(),
2118+
diag::dynamic_replacement_accessor_ambiguous, replacedVarName);
2119+
for (auto result : results) {
2120+
TC.diagnose(result,
2121+
diag::dynamic_replacement_accessor_ambiguous_candidate,
2122+
result->getModuleContext()->getFullName());
2123+
}
2124+
attr->setInvalid();
2125+
return nullptr;
2126+
}
20702127

20712128
assert(!isa<FuncDecl>(results[0]));
20722129
TC.validateDecl(results[0]);
@@ -2117,6 +2174,10 @@ findReplacedFunction(DeclName replacedFunctionName,
21172174
lookupReplacedDecl(replacedFunctionName, attr, replacement, results);
21182175

21192176
for (auto *result : results) {
2177+
// Check for static/instance mismatch.
2178+
if (result->isStatic() != replacement->isStatic())
2179+
continue;
2180+
21202181
TC.validateDecl(result);
21212182
if (result->getInterfaceType()->getCanonicalType()->matches(
21222183
replacement->getInterfaceType()->getCanonicalType(),
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
public struct TheReplaceables {
2+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import A
2+
3+
public extension TheReplaceables {
4+
dynamic var property1: Int { return 0 }
5+
dynamic var property2: Int { return 0 }
6+
7+
dynamic subscript (i: Int) -> Int { return 0 }
8+
dynamic subscript (i: Int) -> String { return "" }
9+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import A
2+
3+
public extension TheReplaceables {
4+
dynamic var property1: Int { return 0 }
5+
dynamic var property2: String { return "" }
6+
7+
dynamic subscript (i: Int) -> Int { return 0 }
8+
dynamic subscript (s: String) -> String { return "" }
9+
}

test/attr/dynamicReplacement.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic %S/Inputs/dynamicReplacementA.swift -o %t -module-name A
3+
// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic -c %S/Inputs/dynamicReplacementB.swift -o %t -I %t -module-name B
4+
// RUN: %target-swift-frontend -emit-module -swift-version 5 -enable-implicit-dynamic -c %S/Inputs/dynamicReplacementC.swift -o %t -I %t -module-name C
5+
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-implicit-dynamic -I %t
6+
import A
7+
import B
8+
import C
9+
10+
// rdar://problem/46737657: static properties
11+
struct StaticProperties {
12+
dynamic var property: Int { return 1 }
13+
dynamic static var property: Int { return 11 }
14+
}
15+
16+
extension StaticProperties {
17+
@_dynamicReplacement(for: property)
18+
var replacement_property: Int { return 2 }
19+
}
20+
21+
// Replacements involving different types.
22+
extension TheReplaceables {
23+
@_dynamicReplacement(for: property1) // expected-error{{replaced accessor for 'property1' occurs in multiple places}}
24+
var replace_property1: Int { return 0 }
25+
26+
@_dynamicReplacement(for: property2)
27+
var replace_property2_int: Int { return 1 }
28+
29+
@_dynamicReplacement(for: property2)
30+
var replace_property2_string: String { return "replaced" }
31+
32+
@_dynamicReplacement(for: subscript(_:)) // expected-error{{replaced accessor for 'subscript(_:)' occurs in multiple places}}
33+
subscript (int_int i: Int) -> Int { return 0 }
34+
35+
@_dynamicReplacement(for: subscript(_:))
36+
subscript (int_string i: Int) -> String { return "" }
37+
38+
@_dynamicReplacement(for: subscript(_:))
39+
subscript (string_string i: String) -> String { return "" }
40+
}

0 commit comments

Comments
 (0)