Skip to content

Commit 708bac7

Browse files
committed
[Sema] Always consider outer alternatives when resolving a decl ref.
Currently, when a type member has the same base name as a global declaration, one cannot refer to the global member from a method context, and the current diagnostics force the user to use a fully qualified name. This is a long-standing issue with the use of top-level `min` and `max` in an extension, and is causing unnecessarily verbosity in math algorithms in extensions for `ElementaryFunctions`-conforming types. More context: 1. [Forum thread](https://forums.swift.org/t/elementaryfunctions-and-ambiguity-in-extensions/25297): `ElementaryFunctions` and ambiguity in extensions 2. [SR-456](https://bugs.swift.org/browse/SR-456): Confusing build error when calling 'max' function within 'extension Int'. 3. [SR-1772](https://bugs.swift.org/browse/SR-1772): File-level function with the same name as instance function not picked up by compiler. This PR makes `UnresolvedDeclRefExpr` resolution always consider alternative candidates from the outer context. The following code will no longer trigger any error or warning. ```swift import Foundation // which exports a top-level `sin(_:)`. extension Float { func foo() { // Refers to top-level `sin(_:)`, not static method `Float.sin(_:)`. _ = sin(self) _ = sin(_:) as (Float) -> Float } } extension Sequence { func foo() { // Refers to top-level `max(_:_:)`. _ = max(1, 2) } } ```
1 parent 2bc397b commit 708bac7

File tree

3 files changed

+15
-44
lines changed

3 files changed

+15
-44
lines changed

lib/Sema/CSApply.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,10 +2660,7 @@ namespace {
26602660
//
26612661
// The only way to get here is via an UnresolvedDotExpr with outer
26622662
// alternatives.
2663-
auto UDE = cast<UnresolvedDotExpr>(expr);
2664-
cs.diagnoseDeprecatedConditionalConformanceOuterAccess(
2665-
UDE, selected.choice.getDecl());
2666-
2663+
assert(isa<UnresolvedDotExpr>(expr));
26672664
return buildDeclRef(selected.choice, nameLoc, selected.openedFullType,
26682665
memberLocator, implicit,
26692666
selected.choice.getFunctionRefKind(),

lib/Sema/CSSimplify.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4604,16 +4604,16 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
46044604
[&](unsigned, const OverloadChoice &choice) {
46054605
return fixMemberRef(*this, baseTy, member, choice, locator);
46064606
});
4607+
}
46074608

4608-
if (!outerAlternatives.empty()) {
4609-
// If local scope has a single choice,
4610-
// it should always be preferred.
4611-
if (candidates.size() == 1)
4612-
candidates.front()->setFavored();
4609+
if (!outerAlternatives.empty()) {
4610+
// If local scope has a single choice,
4611+
// it should always be preferred.
4612+
if (candidates.size() == 1)
4613+
candidates.front()->setFavored();
46134614

4614-
generateConstraints(candidates, memberTy, outerAlternatives,
4615-
useDC, locator);
4616-
}
4615+
generateConstraints(candidates, memberTy, outerAlternatives,
4616+
useDC, locator);
46174617
}
46184618

46194619
if (!result.UnviableCandidates.empty()) {

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -449,20 +449,6 @@ static bool findNonMembers(TypeChecker &TC,
449449
return AllDeclRefs;
450450
}
451451

452-
/// Whether we should be looking at the outer results for a function called \c
453-
/// name.
454-
///
455-
/// This is very restrictive because it's a source compatibility issue (see the
456-
/// if (AllConditionalConformances) { (void)findNonMembers(...); } below).
457-
static bool shouldConsiderOuterResultsFor(DeclName name) {
458-
const StringRef specialNames[] = {"min", "max"};
459-
for (auto specialName : specialNames)
460-
if (name.isSimpleName(specialName))
461-
return true;
462-
463-
return false;
464-
}
465-
466452
/// Bind an UnresolvedDeclRefExpr by performing name lookup and
467453
/// returning the resultant expression. Context is the DeclContext used
468454
/// for the lookup.
@@ -473,11 +459,10 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
473459
SourceLoc Loc = UDRE->getLoc();
474460

475461
// Perform standard value name lookup.
476-
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
462+
NameLookupOptions lookupOptions =
463+
defaultUnqualifiedLookupOptions | NameLookupFlags::IncludeOuterResults;
477464
if (isa<AbstractFunctionDecl>(DC))
478465
lookupOptions |= NameLookupFlags::KnownPrivate;
479-
if (shouldConsiderOuterResultsFor(Name))
480-
lookupOptions |= NameLookupFlags::IncludeOuterResults;
481466

482467
auto Lookup = lookupUnqualified(DC, Name, Loc, lookupOptions);
483468

@@ -762,22 +747,11 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
762747
/*Implicit=*/true);
763748
}
764749

765-
// We *might* include any non-members that we found in outer contexts in
766-
// some special cases, for backwards compatibility: first, we have to be
767-
// looking for one of the special names
768-
// ('shouldConsiderOuterResultsFor(Name)'), and second, all of the inner
769-
// results need to come from conditional conformances. The second condition
770-
// is how the problem here was encountered: a type ('Range') was made to
771-
// conditionally conform to a new protocol ('Sequence'), which introduced
772-
// some extra methods ('min' and 'max') that shadowed global functions that
773-
// people regularly called within extensions to that type (usually adding
774-
// 'clamp').
750+
// Include any non-members that we found in outer contexts.
775751
llvm::SmallVector<ValueDecl *, 4> outerAlternatives;
776-
if (AllConditionalConformances) {
777-
(void)findNonMembers(*this, Lookup.outerResults(), UDRE->getRefKind(),
778-
/*breakOnMember=*/false, outerAlternatives,
779-
/*isValid=*/[&](ValueDecl *) { return true; });
780-
}
752+
(void)findNonMembers(*this, Lookup.outerResults(), UDRE->getRefKind(),
753+
/*breakOnMember=*/false, outerAlternatives,
754+
/*isValid=*/[&](ValueDecl *) { return true; });
781755

782756
// Otherwise, form an UnresolvedDotExpr and sema will resolve it based on
783757
// type information.

0 commit comments

Comments
 (0)