Skip to content

[Sema] Always consider outer alternatives when resolving a decl ref. #25316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Sema] Always consider outer alternatives when resolving a decl ref. #25316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2660,10 +2660,7 @@ namespace {
//
// The only way to get here is via an UnresolvedDotExpr with outer
// alternatives.
auto UDE = cast<UnresolvedDotExpr>(expr);
cs.diagnoseDeprecatedConditionalConformanceOuterAccess(
UDE, selected.choice.getDecl());

assert(isa<UnresolvedDotExpr>(expr));
return buildDeclRef(selected.choice, nameLoc, selected.openedFullType,
memberLocator, implicit,
selected.choice.getFunctionRefKind(),
Expand Down
16 changes: 8 additions & 8 deletions lib/Sema/CSSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4604,16 +4604,16 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
[&](unsigned, const OverloadChoice &choice) {
return fixMemberRef(*this, baseTy, member, choice, locator);
});
}

if (!outerAlternatives.empty()) {
// If local scope has a single choice,
// it should always be preferred.
if (candidates.size() == 1)
candidates.front()->setFavored();
if (!outerAlternatives.empty()) {
// If local scope has a single choice,
// it should always be preferred.
if (candidates.size() == 1)
candidates.front()->setFavored();

generateConstraints(candidates, memberTy, outerAlternatives,
useDC, locator);
}
generateConstraints(candidates, memberTy, outerAlternatives,
useDC, locator);
}

if (!result.UnviableCandidates.empty()) {
Expand Down
38 changes: 6 additions & 32 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,20 +449,6 @@ static bool findNonMembers(TypeChecker &TC,
return AllDeclRefs;
}

/// Whether we should be looking at the outer results for a function called \c
/// name.
///
/// This is very restrictive because it's a source compatibility issue (see the
/// if (AllConditionalConformances) { (void)findNonMembers(...); } below).
static bool shouldConsiderOuterResultsFor(DeclName name) {
const StringRef specialNames[] = {"min", "max"};
for (auto specialName : specialNames)
if (name.isSimpleName(specialName))
return true;

return false;
}

/// Bind an UnresolvedDeclRefExpr by performing name lookup and
/// returning the resultant expression. Context is the DeclContext used
/// for the lookup.
Expand All @@ -473,11 +459,10 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
SourceLoc Loc = UDRE->getLoc();

// Perform standard value name lookup.
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
NameLookupOptions lookupOptions =
defaultUnqualifiedLookupOptions | NameLookupFlags::IncludeOuterResults;
if (isa<AbstractFunctionDecl>(DC))
lookupOptions |= NameLookupFlags::KnownPrivate;
if (shouldConsiderOuterResultsFor(Name))
lookupOptions |= NameLookupFlags::IncludeOuterResults;

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

Expand Down Expand Up @@ -762,22 +747,11 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
/*Implicit=*/true);
}

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

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