Skip to content

Commit f45222c

Browse files
beccadaxbeccadax
committed
Diagnose module selectors with local vars right
1 parent fd4f8e0 commit f45222c

File tree

9 files changed

+36
-20
lines changed

9 files changed

+36
-20
lines changed

‎include/swift/AST/NameLookup.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,12 +728,12 @@ class ASTScope {
728728
///
729729
/// \param stopAfterInnermostBraceStmt If lookup should consider
730730
/// local declarations inside the innermost syntactic scope only.
731-
static void lookupLocalDecls(SourceFile *, DeclName, SourceLoc,
731+
static void lookupLocalDecls(SourceFile *, DeclNameRef, SourceLoc,
732732
bool stopAfterInnermostBraceStmt,
733733
SmallVectorImpl<ValueDecl *> &);
734734

735735
/// Returns the result if there is exactly one, nullptr otherwise.
736-
static ValueDecl *lookupSingleLocalDecl(SourceFile *, DeclName, SourceLoc);
736+
static ValueDecl *lookupSingleLocalDecl(SourceFile *, DeclNameRef, SourceLoc);
737737

738738
/// Entry point to record the visible statement labels from the given
739739
/// point.

‎lib/AST/UnqualifiedLookup.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -789,19 +789,21 @@ namespace {
789789

790790
class ASTScopeDeclConsumerForLocalLookup
791791
: public AbstractASTScopeDeclConsumer {
792-
DeclName name;
792+
DeclNameRef name;
793793
bool stopAfterInnermostBraceStmt;
794794
SmallVectorImpl<ValueDecl *> &results;
795795

796796
public:
797797
ASTScopeDeclConsumerForLocalLookup(
798-
DeclName name, bool stopAfterInnermostBraceStmt,
798+
DeclNameRef name, bool stopAfterInnermostBraceStmt,
799799
SmallVectorImpl<ValueDecl *> &results)
800800
: name(name), stopAfterInnermostBraceStmt(stopAfterInnermostBraceStmt),
801801
results(results) {}
802802

803803
bool consume(ArrayRef<ValueDecl *> values,
804804
NullablePtr<DeclContext> baseDC) override {
805+
if (name.hasModuleSelector()) return false;
806+
805807
for (auto *value: values) {
806808
bool foundMatch = false;
807809
if (auto *varDecl = dyn_cast<VarDecl>(value)) {
@@ -814,7 +816,7 @@ class ASTScopeDeclConsumerForLocalLookup
814816
});
815817
}
816818

817-
if (!foundMatch && value->getName().matchesRef(name))
819+
if (!foundMatch && value->getName().matchesRef(name.getFullName()))
818820
results.push_back(value);
819821
}
820822

@@ -841,15 +843,15 @@ class ASTScopeDeclConsumerForLocalLookup
841843

842844
/// Lookup that only finds local declarations and does not trigger
843845
/// interface type computation.
844-
void ASTScope::lookupLocalDecls(SourceFile *sf, DeclName name, SourceLoc loc,
846+
void ASTScope::lookupLocalDecls(SourceFile *sf, DeclNameRef name, SourceLoc loc,
845847
bool stopAfterInnermostBraceStmt,
846848
SmallVectorImpl<ValueDecl *> &results) {
847849
ASTScopeDeclConsumerForLocalLookup consumer(name, stopAfterInnermostBraceStmt,
848850
results);
849851
ASTScope::unqualifiedLookup(sf, loc, consumer);
850852
}
851853

852-
ValueDecl *ASTScope::lookupSingleLocalDecl(SourceFile *sf, DeclName name,
854+
ValueDecl *ASTScope::lookupSingleLocalDecl(SourceFile *sf, DeclNameRef name,
853855
SourceLoc loc) {
854856
SmallVector<ValueDecl *, 1> result;
855857
ASTScope::lookupLocalDecls(sf, name, loc,

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1397,7 +1397,7 @@ class VarDeclMultipleReferencesChecker : public ASTWalker {
13971397
if (name.isSimpleName(varDecl->getName()) && loc.isValid()) {
13981398
auto *otherDecl =
13991399
ASTScope::lookupSingleLocalDecl(DC->getParentSourceFile(),
1400-
name.getFullName(), loc);
1400+
name, loc);
14011401
if (otherDecl == varDecl)
14021402
++count;
14031403
}

‎lib/Sema/CSGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2557,7 +2557,7 @@ namespace {
25572557
if (name.isSimpleName() && loc.isValid()) {
25582558
auto *varDecl = dyn_cast_or_null<VarDecl>(
25592559
ASTScope::lookupSingleLocalDecl(cs.DC->getParentSourceFile(),
2560-
name.getFullName(), loc));
2560+
name, loc));
25612561
if (varDecl)
25622562
if (auto varType = cs.getTypeIfAvailable(varDecl))
25632563
varType->getTypeVariables(varRefs);

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3279,7 +3279,7 @@ void VarDeclUsageChecker::handleIfConfig(IfConfigDecl *ICD) {
32793279
auto loc = declRef->getLoc();
32803280
if (name.isSimpleName() && loc.isValid()) {
32813281
auto *varDecl = dyn_cast_or_null<VarDecl>(
3282-
ASTScope::lookupSingleLocalDecl(SF, name.getFullName(), loc));
3282+
ASTScope::lookupSingleLocalDecl(SF, name, loc));
32833283
if (varDecl)
32843284
VDUC.addMark(varDecl, RK_Read|RK_Written);
32853285
}

‎lib/Sema/PreCheckExpr.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,8 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
406406
}
407407

408408
DeclName lookupName(context, Name.getBaseName(), lookupLabels);
409-
LookupName = DeclNameRef(lookupName);
409+
LookupName = DeclNameRef(DC->getASTContext(), Name.getModuleSelector(),
410+
lookupName);
410411
}
411412

412413
auto errorResult = [&]() -> Expr * {
@@ -432,8 +433,7 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
432433
// First, look for a local binding in scope.
433434
if (Loc.isValid() && !Name.isOperator() && !Name.hasModuleSelector()) {
434435
SmallVector<ValueDecl *, 2> localDecls;
435-
ASTScope::lookupLocalDecls(DC->getParentSourceFile(),
436-
LookupName.getFullName(), Loc,
436+
ASTScope::lookupLocalDecls(DC->getParentSourceFile(), LookupName, Loc,
437437
/*stopAfterInnermostBraceStmt=*/false,
438438
ResultValues);
439439
for (auto *localDecl : ResultValues) {
@@ -524,7 +524,8 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
524524
// It's just something we need to pick up contextually.
525525
if (decl->getDeclContext()->getLocalContext())
526526
decl->diagnose(diag::note_remove_module_selector)
527-
.fixItRemove(moduleSelectorLoc);
527+
.fixItRemoveChars(moduleSelectorLoc,
528+
UDRE->getNameLoc().getBaseNameLoc());
528529

529530
if (decl->isInstanceMember())
530531
Context.Diags.diagnose(moduleSelectorLoc,
@@ -1090,8 +1091,9 @@ namespace {
10901091

10911092
// Do an actual lookup for 'self' in case it shows up in a capture list.
10921093
auto *methodSelf = methodContext->getImplicitSelfDecl();
1093-
auto *lookupSelf = ASTScope::lookupSingleLocalDecl(DC->getParentSourceFile(),
1094-
Ctx.Id_self, Loc);
1094+
auto *lookupSelf =
1095+
ASTScope::lookupSingleLocalDecl(DC->getParentSourceFile(),
1096+
DeclNameRef(Ctx.Id_self), Loc);
10951097
if (lookupSelf && lookupSelf != methodSelf) {
10961098
// FIXME: This is the wrong diagnostic for if someone manually declares a
10971099
// variable named 'self' using backticks.

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,8 @@ BodyInitKindRequest::evaluate(Evaluator &evaluator,
526526
auto loc = declRef->getLoc();
527527
if (name.isSimpleName(ctx.Id_self)) {
528528
auto *otherSelfDecl =
529-
ASTScope::lookupSingleLocalDecl(Decl->getParentSourceFile(),
530-
name.getFullName(), loc);
529+
ASTScope::lookupSingleLocalDecl(Decl->getParentSourceFile(), name,
530+
loc);
531531
if (otherSelfDecl == Decl->getImplicitSelfDecl())
532532
myKind = BodyInitKind::Delegating;
533533
}

‎lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,8 @@ CheckRedeclarationRequest::evaluate(Evaluator &eval, ValueDecl *current) const {
547547
}
548548
} else if (currentDC->isLocalContext()) {
549549
if (!current->isImplicit()) {
550-
ASTScope::lookupLocalDecls(currentFile, current->getBaseName(),
550+
ASTScope::lookupLocalDecls(currentFile,
551+
DeclNameRef(current->getBaseName()),
551552
current->getLoc(),
552553
/*stopAfterInnermostBraceStmt=*/true,
553554
otherDefinitions);

‎test/NameLookup/module_selector.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ extension D: Swift::Equatable {
232232
// expected-error@-4 {{expected expression after operator}}
233233
let magnitude: Int.Swift::Magnitude = Swift::magnitude
234234
// expected-error@-1 {{declaration 'magnitude' is not imported through module 'Swift'}}
235-
// FIXME should be un-addressable via main: expected-note@-2 {{did you mean module 'main'?}}
235+
// expected-note@-2 {{did you mean module 'main'?}}
236236
if Swift::Bool.Swift::random() {
237237
Swift::negate()
238238
// expected-error@-1 {{declaration 'negate' is not imported through module 'Swift'}}
@@ -257,6 +257,17 @@ extension D: Swift::Equatable {
257257
// FIXME: Can we test @convention(witness_method:)?
258258
}
259259

260+
let mog: Never = fatalError()
261+
262+
func localVarsCantBeAccessedByModuleSelector() {
263+
let mag: Int.Swift::Magnitude = main::mag
264+
// expected-error@-1 {{declaration 'mag' is not imported through module 'main'}}
265+
// expected-note@-2 {{did you mean the local declaration?}} {{35-41=}}
266+
267+
let mog: Never = main::mog
268+
// no-error
269+
}
270+
260271
struct AvailableUser {
261272
@available(macOS 10.15, *) var use1: String { "foo" }
262273

0 commit comments

Comments
 (0)