Skip to content

Commit 60842b5

Browse files
committed
[ConstraintSystem] Increase score only if members found on Optional and its unwrapped type
Unresolved member lookup is allowed to perform implicit optional unwrap of a base type to find members. Previously if there were any members directly on `Optional`, lookup would stop there. But since SR-13815 it became possible for solver to attempt members found on unwrapped type even if there are viable ones on `Optional` as well. New score kind has been introduced to guard against possible ambiguities with new scheme - `SK_UnresolvedMemberViaOptional`. It's used very time member found via base type unwrap is attempted. Unfortunately, doing so can lead to behavior changes in existing code because it's possible that base was wrapped into optional implicitly based on context e.g. unresolved member passed in as an argument to a parameter of optional type. To fix situations like that, `SK_UnresolvedMemberViaOptional` should only be increased if there is a mix of members to attempt - both directly on `Optional` and on unwrapped type, in all other cases score should stay the same because there could be no ambiguity. Resolves: rdar://73027153 (cherry picked from commit 51dc7fdefab230d76518b97f9074a3f98e1fd0c3)
1 parent 4070b4f commit 60842b5

File tree

4 files changed

+62
-13
lines changed

4 files changed

+62
-13
lines changed

include/swift/Sema/OverloadChoice.h

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,14 @@ class OverloadChoice {
7373
/// optional context type, turning a "Decl" kind into
7474
/// "DeclViaUnwrappedOptional".
7575
IsDeclViaUnwrappedOptional = 0x02,
76+
/// Indicates that there are viable members found on `Optional`
77+
/// type and its underlying type. And current overload choice
78+
/// is a backup one, which should be picked only if members
79+
/// found directly on `Optional` do not match.
80+
IsFallbackDeclViaUnwrappedOptional = 0x03,
7681
/// Indicates that this declaration was dynamic, turning a
7782
/// "Decl" kind into "DeclViaDynamic" kind.
78-
IsDeclViaDynamic = 0x03,
83+
IsDeclViaDynamic = 0x07,
7984
};
8085

8186
/// The base type to be used when referencing the declaration
@@ -175,17 +180,23 @@ class OverloadChoice {
175180

176181
/// Retrieve an overload choice for a declaration that was found
177182
/// by unwrapping an optional context type.
183+
///
184+
/// \param isFallback Indicates that this result should be used
185+
/// as a backup, if member found directly on `Optional` doesn't
186+
/// match.
178187
static OverloadChoice
179-
getDeclViaUnwrappedOptional(Type base, ValueDecl *value,
188+
getDeclViaUnwrappedOptional(Type base, ValueDecl *value, bool isFallback,
180189
FunctionRefKind functionRefKind) {
181190
OverloadChoice result;
182191
result.BaseAndDeclKind.setPointer(base);
183-
result.BaseAndDeclKind.setInt(IsDeclViaUnwrappedOptional);
192+
result.BaseAndDeclKind.setInt(isFallback
193+
? IsFallbackDeclViaUnwrappedOptional
194+
: IsDeclViaUnwrappedOptional);
184195
result.DeclOrKind = value;
185196
result.TheFunctionRefKind = functionRefKind;
186197
return result;
187198
}
188-
199+
189200
/// Retrieve an overload choice for a declaration that was found via
190201
/// dynamic member lookup. The `ValueDecl` is a `subscript(dynamicMember:)`
191202
/// method.
@@ -219,6 +230,7 @@ class OverloadChoice {
219230
case IsDeclViaBridge: return OverloadChoiceKind::DeclViaBridge;
220231
case IsDeclViaDynamic: return OverloadChoiceKind::DeclViaDynamic;
221232
case IsDeclViaUnwrappedOptional:
233+
case IsFallbackDeclViaUnwrappedOptional:
222234
return OverloadChoiceKind::DeclViaUnwrappedOptional;
223235
default: return OverloadChoiceKind::Decl;
224236
}
@@ -256,6 +268,12 @@ class OverloadChoice {
256268
return getKind() == OverloadChoiceKind::KeyPathDynamicMemberLookup;
257269
}
258270

271+
/// Determine whether this member is a backup in case
272+
/// members found directly on `Optional` didn't match.
273+
bool isFallbackMemberOnUnwrappedBase() const {
274+
return BaseAndDeclKind.getInt() == IsFallbackDeclViaUnwrappedOptional;
275+
}
276+
259277
/// Get the name of the overload choice.
260278
DeclName getName() const;
261279

lib/Sema/CSSimplify.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6666,8 +6666,9 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
66666666

66676667
// Local function that turns a ValueDecl into a properly configured
66686668
// OverloadChoice.
6669-
auto getOverloadChoice = [&](ValueDecl *cand, bool isBridged,
6670-
bool isUnwrappedOptional) -> OverloadChoice {
6669+
auto getOverloadChoice =
6670+
[&](ValueDecl *cand, bool isBridged, bool isUnwrappedOptional,
6671+
bool isFallbackUnwrap = false) -> OverloadChoice {
66716672
// If we're looking into an existential type, check whether this
66726673
// result was found via dynamic lookup.
66736674
if (instanceTy->isAnyObject()) {
@@ -6688,8 +6689,9 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
66886689
auto ovlBaseTy = MetatypeType::get(baseTy->castTo<MetatypeType>()
66896690
->getInstanceType()
66906691
->getOptionalObjectType());
6691-
return OverloadChoice::getDeclViaUnwrappedOptional(ovlBaseTy, cand,
6692-
functionRefKind);
6692+
return OverloadChoice::getDeclViaUnwrappedOptional(
6693+
ovlBaseTy, cand,
6694+
/*isFallback=*/isFallbackUnwrap, functionRefKind);
66936695
}
66946696

66956697
// While looking for subscript choices it's possible to find
@@ -6713,7 +6715,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
67136715

67146716
return OverloadChoice(baseTy, cand, functionRefKind);
67156717
};
6716-
6718+
67176719
// Add all results from this lookup.
67186720
for (auto result : lookup)
67196721
addChoice(getOverloadChoice(result.getValueDecl(),
@@ -6796,11 +6798,16 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
67966798
}
67976799

67986800
if (objectType->mayHaveMembers()) {
6801+
// If there are viable members directly on `Optional`, let's
6802+
// prioritize them any mark any results found on wrapped type
6803+
// as a fallback results.
6804+
bool isFallback = !result.ViableCandidates.empty();
67996805
LookupResult &optionalLookup = lookupMember(objectType, memberName);
68006806
for (auto result : optionalLookup)
68016807
addChoice(getOverloadChoice(result.getValueDecl(),
6802-
/*bridged*/false,
6803-
/*isUnwrappedOptional=*/true));
6808+
/*bridged*/ false,
6809+
/*isUnwrappedOptional=*/true,
6810+
/*isUnwrapFallback=*/isFallback));
68046811
}
68056812
}
68066813
}

lib/Sema/ConstraintSystem.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2798,8 +2798,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
27982798
increaseScore(SK_DisfavoredOverload);
27992799
}
28002800

2801-
if (choice.getKind() == OverloadChoiceKind::DeclViaUnwrappedOptional &&
2802-
locator->isLastElement<LocatorPathElt::UnresolvedMember>()) {
2801+
if (choice.isFallbackMemberOnUnwrappedBase()) {
28032802
increaseScore(SK_UnresolvedMemberViaOptional);
28042803
}
28052804
}

test/Constraints/sr13815.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
// SR-13815
4+
5+
enum E {
6+
case foo(String)
7+
}
8+
9+
struct Test {
10+
var bar: E?
11+
}
12+
13+
struct S {
14+
func evaluate(_: Test) -> [Test] {
15+
return []
16+
}
17+
18+
func test(set: Set<String>) {
19+
let flattened = set.flatMap { element in
20+
evaluate(Test(bar: .foo(element)))
21+
}
22+
23+
let _: [Test] = flattened // Ok (find .`bar` after unwrap)
24+
}
25+
}

0 commit comments

Comments
 (0)