Skip to content

Sema: Fix a MemberImportVisibility regression in resolveDeclRefExpr() #84659

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

Merged
merged 1 commit into from
Oct 4, 2025
Merged

Sema: Fix a MemberImportVisibility regression in resolveDeclRefExpr() #84659

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
301 changes: 164 additions & 137 deletions lib/Sema/PreCheckTarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,12 @@ diagnoseUnqualifiedInit(UnresolvedDeclRefExpr *initExpr, DeclContext *dc,
initExpr->getNameLoc(), /*implicit=*/true);
}

/// Bind an UnresolvedDeclRefExpr by performing name lookup and
/// returning the resultant expression. Context is the DeclContext used
/// for the lookup.
Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
DeclContext *DC) {
/// Bind an UnresolvedDeclRefExpr by performing name lookup using the given
/// options and returning the resultant expression. `DC` is the DeclContext
/// used for the lookup. If the lookup doesn't find any results, returns
/// `nullptr`.
static Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC,
NameLookupOptions lookupOptions) {
auto &Context = DC->getASTContext();
DeclNameRef Name = UDRE->getName();
SourceLoc Loc = UDRE->getLoc();
Expand Down Expand Up @@ -562,14 +563,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
LookupName = DeclNameRef(lookupName);
}

// Perform standard value name lookup.
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
// TODO: Include all of the possible members to give a solver a
// chance to diagnose name shadowing which requires explicit
// name/module qualifier to access top-level name.
lookupOptions |= NameLookupFlags::IncludeOuterResults;
lookupOptions |= NameLookupFlags::IgnoreMissingImports;

LookupResult Lookup;

bool AllDeclRefs = true;
Expand Down Expand Up @@ -628,17 +621,8 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
}

if (!Lookup) {
// If we failed lookup of an operator, check to see if this is a range
// operator misspelling. Otherwise try to diagnose a juxtaposition
// e.g. (x*-4) that needs whitespace.
if (diagnoseRangeOperatorMisspell(Context.Diags, UDRE) ||
diagnoseIncDecOperator(Context.Diags, UDRE) ||
diagnoseOperatorJuxtaposition(UDRE, DC) ||
diagnoseNonexistentPowerOperator(Context.Diags, UDRE, DC)) {
return errorResult();
}

// Try ignoring access control.
// For the purpose of diagnosing inaccessible results, try the lookup again
// but ignore access control.
NameLookupOptions relookupOptions = lookupOptions;
relookupOptions |= NameLookupFlags::IgnoreAccessControl;
auto inaccessibleResults =
Expand All @@ -663,117 +647,8 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
return errorResult();
}

// TODO: Name will be a compound name if it was written explicitly as
// one, but we should also try to propagate labels into this.
DeclNameLoc nameLoc = UDRE->getNameLoc();

Identifier simpleName = Name.getBaseIdentifier();
const char *buffer = simpleName.get();
llvm::SmallString<64> expectedIdentifier;
bool isConfused = false;
uint32_t codepoint;
uint32_t firstConfusableCodepoint = 0;
int totalCodepoints = 0;
int offset = 0;
while ((codepoint = validateUTF8CharacterAndAdvance(buffer,
buffer +
strlen(buffer)))
!= ~0U) {
int length = (buffer - simpleName.get()) - offset;
if (auto expectedCodepoint =
confusable::tryConvertConfusableCharacterToASCII(codepoint)) {
if (firstConfusableCodepoint == 0) {
firstConfusableCodepoint = codepoint;
}
isConfused = true;
expectedIdentifier += expectedCodepoint;
} else {
expectedIdentifier += (char)codepoint;
}

totalCodepoints++;

offset += length;
}

auto emitBasicError = [&] {

if (Name.isSimpleName(Context.Id_self)) {
// `self` gets diagnosed with a different error when it can't be found.
Context.Diags
.diagnose(Loc, diag::cannot_find_self_in_scope)
.highlight(UDRE->getSourceRange());
} else {
Context.Diags
.diagnose(Loc, diag::cannot_find_in_scope, Name,
Name.isOperator())
.highlight(UDRE->getSourceRange());
}

if (!Context.LangOpts.DisableExperimentalClangImporterDiagnostics) {
Context.getClangModuleLoader()->diagnoseTopLevelValue(
Name.getFullName());
}
};

if (!isConfused) {
if (Name.isSimpleName(Context.Id_Self)) {
if (DeclContext *typeContext = DC->getInnermostTypeContext()){
Type SelfType = typeContext->getSelfInterfaceType();

if (typeContext->getSelfClassDecl() &&
!typeContext->getSelfClassDecl()->isForeignReferenceType())
SelfType = DynamicSelfType::get(SelfType, Context);
return new (Context)
TypeExpr(new (Context) SelfTypeRepr(SelfType, Loc));
}
}

TypoCorrectionResults corrections(Name, nameLoc);

// FIXME: Don't perform typo correction inside macro arguments, because it
// will invoke synthesizing declarations in this scope, which will attempt to
// expand this macro which leads to circular reference errors.
if (!namelookup::isInMacroArgument(DC->getParentSourceFile(), UDRE->getLoc())) {
TypeChecker::performTypoCorrection(DC, UDRE->getRefKind(), Type(),
lookupOptions, corrections);
}

if (auto typo = corrections.claimUniqueCorrection()) {
auto diag = Context.Diags.diagnose(
Loc, diag::cannot_find_in_scope_corrected, Name,
Name.isOperator(), typo->CorrectedName.getBaseIdentifier().str());
diag.highlight(UDRE->getSourceRange());
typo->addFixits(diag);
} else {
emitBasicError();
}

corrections.noteAllCandidates();
} else {
emitBasicError();

if (totalCodepoints == 1) {
auto charNames = confusable::getConfusableAndBaseCodepointNames(
firstConfusableCodepoint);
Context.Diags
.diagnose(Loc, diag::single_confusable_character,
UDRE->getName().isOperator(), simpleName.str(),
charNames.first, expectedIdentifier, charNames.second)
.fixItReplace(Loc, expectedIdentifier);
} else {
Context.Diags
.diagnose(Loc, diag::confusable_character,
UDRE->getName().isOperator(), simpleName.str(),
expectedIdentifier)
.fixItReplace(Loc, expectedIdentifier);
}
}

// TODO: consider recovering from here. We may want some way to suppress
// downstream diagnostics, though.

return errorResult();
// No results were found.
return nullptr;
}

// FIXME: Need to refactor the way we build an AST node from a lookup result!
Expand Down Expand Up @@ -892,8 +767,9 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
UDRE->getNameLoc().getStartLoc());
}

return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
UDRE->isImplicit(), UDRE->getFunctionRefInfo());
return TypeChecker::buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
UDRE->isImplicit(),
UDRE->getFunctionRefInfo());
}

ResultValues.clear();
Expand Down Expand Up @@ -983,6 +859,157 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
return errorResult();
}

Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
DeclContext *DC) {
auto &Context = DC->getASTContext();

// Perform standard value name lookup.
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
// TODO: Include all of the possible members to give a solver a
// chance to diagnose name shadowing which requires explicit
// name/module qualifier to access top-level name.
lookupOptions |= NameLookupFlags::IncludeOuterResults;

Expr *result = ::resolveDeclRefExpr(UDRE, DC, lookupOptions);
if (!result && Context.LangOpts.hasFeature(Feature::MemberImportVisibility,
/*allowMigration=*/true)) {
// If we didn't find a result, try again but this time relax
// MemberImportVisibility restrictions. Note that diagnosing the missing
// import is already handled by resolveDeclRefExpr().
lookupOptions |= NameLookupFlags::IgnoreMissingImports;
result = ::resolveDeclRefExpr(UDRE, DC, lookupOptions);
}

if (result)
return result;

// We didn't find any results. Look for common mistakes to diagnose.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the code below this point just moved but is otherwise unchanged.

DeclNameRef Name = UDRE->getName();
SourceLoc Loc = UDRE->getLoc();
if (Loc.isInvalid())
DC = DC->getModuleScopeContext();

auto errorResult = [&]() -> Expr * {
return new (Context) ErrorExpr(UDRE->getSourceRange());
};

// If we failed lookup of an operator, check to see if this is a range
// operator misspelling. Otherwise try to diagnose a juxtaposition
// e.g. (x*-4) that needs whitespace.
if (diagnoseRangeOperatorMisspell(Context.Diags, UDRE) ||
diagnoseIncDecOperator(Context.Diags, UDRE) ||
diagnoseOperatorJuxtaposition(UDRE, DC) ||
diagnoseNonexistentPowerOperator(Context.Diags, UDRE, DC)) {
return errorResult();
}

// TODO: Name will be a compound name if it was written explicitly as
// one, but we should also try to propagate labels into this.
DeclNameLoc nameLoc = UDRE->getNameLoc();

Identifier simpleName = Name.getBaseIdentifier();
const char *buffer = simpleName.get();
llvm::SmallString<64> expectedIdentifier;
bool isConfused = false;
uint32_t codepoint;
uint32_t firstConfusableCodepoint = 0;
int totalCodepoints = 0;
int offset = 0;
while ((codepoint = validateUTF8CharacterAndAdvance(
buffer, buffer + strlen(buffer))) != ~0U) {
int length = (buffer - simpleName.get()) - offset;
if (auto expectedCodepoint =
confusable::tryConvertConfusableCharacterToASCII(codepoint)) {
if (firstConfusableCodepoint == 0) {
firstConfusableCodepoint = codepoint;
}
isConfused = true;
expectedIdentifier += expectedCodepoint;
} else {
expectedIdentifier += (char)codepoint;
}

totalCodepoints++;

offset += length;
}

auto emitBasicError = [&] {
if (Name.isSimpleName(Context.Id_self)) {
// `self` gets diagnosed with a different error when it can't be found.
Context.Diags.diagnose(Loc, diag::cannot_find_self_in_scope)
.highlight(UDRE->getSourceRange());
} else {
Context.Diags
.diagnose(Loc, diag::cannot_find_in_scope, Name, Name.isOperator())
.highlight(UDRE->getSourceRange());
}

if (!Context.LangOpts.DisableExperimentalClangImporterDiagnostics) {
Context.getClangModuleLoader()->diagnoseTopLevelValue(Name.getFullName());
}
};

if (!isConfused) {
if (Name.isSimpleName(Context.Id_Self)) {
if (DeclContext *typeContext = DC->getInnermostTypeContext()) {
Type SelfType = typeContext->getSelfInterfaceType();

if (typeContext->getSelfClassDecl() &&
!typeContext->getSelfClassDecl()->isForeignReferenceType())
SelfType = DynamicSelfType::get(SelfType, Context);
return new (Context)
TypeExpr(new (Context) SelfTypeRepr(SelfType, Loc));
}
}

TypoCorrectionResults corrections(Name, nameLoc);

// FIXME: Don't perform typo correction inside macro arguments, because it
// will invoke synthesizing declarations in this scope, which will attempt
// to expand this macro which leads to circular reference errors.
if (!namelookup::isInMacroArgument(DC->getParentSourceFile(),
UDRE->getLoc())) {
TypeChecker::performTypoCorrection(DC, UDRE->getRefKind(), Type(),
lookupOptions, corrections);
}

if (auto typo = corrections.claimUniqueCorrection()) {
auto diag = Context.Diags.diagnose(
Loc, diag::cannot_find_in_scope_corrected, Name, Name.isOperator(),
typo->CorrectedName.getBaseIdentifier().str());
diag.highlight(UDRE->getSourceRange());
typo->addFixits(diag);
} else {
emitBasicError();
}

corrections.noteAllCandidates();
} else {
emitBasicError();

if (totalCodepoints == 1) {
auto charNames = confusable::getConfusableAndBaseCodepointNames(
firstConfusableCodepoint);
Context.Diags
.diagnose(Loc, diag::single_confusable_character,
UDRE->getName().isOperator(), simpleName.str(),
charNames.first, expectedIdentifier, charNames.second)
.fixItReplace(Loc, expectedIdentifier);
} else {
Context.Diags
.diagnose(Loc, diag::confusable_character,
UDRE->getName().isOperator(), simpleName.str(),
expectedIdentifier)
.fixItReplace(Loc, expectedIdentifier);
}
}

// TODO: consider recovering from here. We may want some way to suppress
// downstream diagnostics, though.
return errorResult();
}

/// If an expression references 'self.init' or 'super.init' in an
/// initializer context, returns the implicit 'self' decl of the constructor.
/// Otherwise, return nil.
Expand Down
21 changes: 21 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_A.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ infix operator <<<
infix operator >>>
infix operator <>

@available(*, deprecated)
public func shadowedByMemberOnXinA() { }

@available(*, deprecated)
public func shadowedByMemberOnXinB() { }

@available(*, deprecated)
public func shadowedByMemberOnXinC() { }

@available(*, deprecated)
public func shadowedByStaticMemberOnXinA() { }

@available(*, deprecated)
public func shadowedByStaticMemberOnXinB() { }

@available(*, deprecated)
public func shadowedByStaticMemberOnXinC() { }

extension X {
public func XinA() { }

Expand All @@ -28,6 +46,9 @@ extension X {

public struct NestedInA {}
public protocol ProtoNestedInA {}

public func shadowedByMemberOnXinA() { }
public static func shadowedByStaticMemberOnXinA() { }
}

extension Y {
Expand Down
5 changes: 5 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_B.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ extension X {
public var propXinB: Bool { return true }
package var propXinB_package: Bool { return true }

public func shadowedByMemberOnXinB() { }
public static func shadowedByStaticMemberOnXinB() { }

public static var max: Int { return Int.min }

public static func >>>(a: Self, b: Self) -> Self { b }

public struct NestedInB {}
Expand Down
3 changes: 3 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_C.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ extension X {

public var propXinC: Bool { return true }

public func shadowedByMemberOnXinC() { }
public static func shadowedByStaticMemberOnXinC() { }

public static func <>(a: Self, b: Self) -> Self { a }

public struct NestedInC {}
Expand Down
Loading