Skip to content

Commit 9f29e97

Browse files
committed
[5.8][Index] Resolve any shadowed references to the decl they shadow
Skip the shadowing decl and instead reference the top-most shadowed decl (ie. follow any shadowed decls up the chain). Also fix `getShorthandShadows` missing references in `MemberDeclRefExpr` by using `getReferencedDecl` instead of assuming it is a `DeclRefExpr`. Resolves rdar://99730146, rdar://100006415, and rdar://103449038. (cherry picked from commit 7b4ab8d)
1 parent 939c8f5 commit 9f29e97

File tree

3 files changed

+140
-28
lines changed

3 files changed

+140
-28
lines changed

lib/IDE/IDETypeChecking.cpp

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -918,35 +918,30 @@ SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
918918
swift::getShorthandShadows(CaptureListExpr *CaptureList, DeclContext *DC) {
919919
SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1> Result;
920920
for (auto Capture : CaptureList->getCaptureList()) {
921-
if (Capture.PBD->getPatternList().size() != 1) {
921+
if (Capture.PBD->getPatternList().size() != 1)
922922
continue;
923-
}
923+
924924
Expr *Init = Capture.PBD->getInit(0);
925-
if (!Init) {
926-
continue;
927-
}
928-
if (auto UDRE = dyn_cast<UnresolvedDeclRefExpr>(Init)) {
929-
if (DC) {
930-
Init = resolveDeclRefExpr(UDRE, DC, /*replaceInvalidRefsWithErrors=*/false);
931-
}
932-
}
933-
auto *DRE = dyn_cast_or_null<DeclRefExpr>(Init);
934-
if (!DRE) {
925+
if (!Init)
935926
continue;
936-
}
937927

938928
auto DeclaredVar = Capture.getVar();
939-
if (DeclaredVar->getLoc() != DRE->getLoc()) {
929+
if (DeclaredVar->getLoc() != Init->getLoc()) {
940930
// We have a capture like `[foo]` if the declared var and the
941931
// reference share the same location.
942932
continue;
943933
}
944934

945-
auto *ReferencedVar = dyn_cast_or_null<VarDecl>(DRE->getDecl());
946-
if (!ReferencedVar) {
947-
continue;
935+
if (auto UDRE = dyn_cast<UnresolvedDeclRefExpr>(Init)) {
936+
if (DC) {
937+
Init = resolveDeclRefExpr(UDRE, DC, /*replaceInvalidRefsWithErrors=*/false);
938+
}
948939
}
949940

941+
auto *ReferencedVar = Init->getReferencedDecl().getDecl();
942+
if (!ReferencedVar)
943+
continue;
944+
950945
assert(DeclaredVar->getName() == ReferencedVar->getName());
951946

952947
Result.emplace_back(std::make_pair(DeclaredVar, ReferencedVar));
@@ -958,28 +953,23 @@ SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
958953
swift::getShorthandShadows(LabeledConditionalStmt *CondStmt, DeclContext *DC) {
959954
SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1> Result;
960955
for (const StmtConditionElement &Cond : CondStmt->getCond()) {
961-
if (Cond.getKind() != StmtConditionElement::CK_PatternBinding) {
956+
if (Cond.getKind() != StmtConditionElement::CK_PatternBinding)
962957
continue;
963-
}
958+
964959
Expr *Init = Cond.getInitializer();
965960
if (auto UDRE = dyn_cast<UnresolvedDeclRefExpr>(Init)) {
966961
if (DC) {
967962
Init = resolveDeclRefExpr(UDRE, DC, /*replaceInvalidRefsWithErrors=*/false);
968963
}
969964
}
970-
auto InitDeclRef = dyn_cast<DeclRefExpr>(Init);
971-
if (!InitDeclRef) {
972-
continue;
973-
}
974-
auto ReferencedVar = dyn_cast_or_null<VarDecl>(InitDeclRef->getDecl());
975-
if (!ReferencedVar) {
965+
966+
auto ReferencedVar = Init->getReferencedDecl().getDecl();
967+
if (!ReferencedVar)
976968
continue;
977-
}
978969

979970
Cond.getPattern()->forEachVariable([&](VarDecl *DeclaredVar) {
980-
if (DeclaredVar->getLoc() != Init->getLoc()) {
971+
if (DeclaredVar->getLoc() != Init->getLoc())
981972
return;
982-
}
983973
assert(DeclaredVar->getName() == ReferencedVar->getName());
984974
Result.emplace_back(std::make_pair(DeclaredVar, ReferencedVar));
985975
});

lib/Index/Index.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/AST/Pattern.h"
2323
#include "swift/AST/ProtocolConformance.h"
2424
#include "swift/AST/SourceFile.h"
25+
#include "swift/AST/Stmt.h"
2526
#include "swift/AST/TypeRepr.h"
2627
#include "swift/AST/Types.h"
2728
#include "swift/AST/USRGeneration.h"
@@ -448,6 +449,11 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
448449
StringScratchSpace stringStorage;
449450
ContainerTracker Containers;
450451

452+
// Contains a mapping for captures of the form [x], from the declared "x"
453+
// to the captured "x" in the enclosing scope. Also includes shorthand if
454+
// let bindings.
455+
llvm::DenseMap<ValueDecl *, ValueDecl *> sameNamedCaptures;
456+
451457
bool getNameAndUSR(ValueDecl *D, ExtensionDecl *ExtD,
452458
StringRef &name, StringRef &USR) {
453459
auto &result = nameAndUSRCache[ExtD ? (Decl*)ExtD : D];
@@ -555,6 +561,16 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
555561
return false;
556562
}
557563

564+
ValueDecl *firstDecl(ValueDecl *D) {
565+
while (true) {
566+
auto captured = sameNamedCaptures.find(D);
567+
if (captured == sameNamedCaptures.end())
568+
break;
569+
D = captured->second;
570+
}
571+
return D;
572+
}
573+
558574
public:
559575
IndexSwiftASTWalker(IndexDataConsumer &IdxConsumer, ASTContext &Ctx,
560576
SourceFile *SF = nullptr)
@@ -701,6 +717,15 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
701717
bool walkToExprPre(Expr *E) override {
702718
if (Cancelled)
703719
return false;
720+
721+
// Record any same named captures/shorthand if let bindings so we can
722+
// treat their references as references to the original decl.
723+
if (auto *captureList = dyn_cast<CaptureListExpr>(E)) {
724+
for (auto shadows : getShorthandShadows(captureList)) {
725+
sameNamedCaptures[shadows.first] = shadows.second;
726+
}
727+
}
728+
704729
ExprStack.push_back(E);
705730
Containers.activateContainersFor(E);
706731
handleMemberwiseInitRefs(E);
@@ -715,6 +740,20 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
715740
return true;
716741
}
717742

743+
bool walkToStmtPre(Stmt *stmt) override {
744+
if (Cancelled)
745+
return false;
746+
747+
// Record any same named captures/shorthand if let bindings so we can
748+
// treat their references as references to the original decl.
749+
if (auto *condition = dyn_cast<LabeledConditionalStmt>(stmt)) {
750+
for (auto shadows : getShorthandShadows(condition)) {
751+
sameNamedCaptures[shadows.first] = shadows.second;
752+
}
753+
}
754+
return true;
755+
}
756+
718757
bool walkToTypeReprPre(TypeRepr *T) override {
719758
if (Cancelled)
720759
return false;
@@ -796,6 +835,11 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
796835
if (isRepressed(Loc) || Loc.isInvalid())
797836
return true;
798837

838+
// Dig back to the original captured variable
839+
if (auto *VD = dyn_cast<VarDecl>(D)) {
840+
D = firstDecl(D);
841+
}
842+
799843
IndexSymbol Info;
800844

801845
if (Data.isImplicit)
@@ -1438,6 +1482,24 @@ bool IndexSwiftASTWalker::reportExtension(ExtensionDecl *D) {
14381482
}
14391483

14401484
bool IndexSwiftASTWalker::report(ValueDecl *D) {
1485+
auto *shadowedDecl = firstDecl(D);
1486+
if (D != shadowedDecl) {
1487+
// Report a reference to the shadowed decl
1488+
SourceLoc loc = D->getNameLoc();
1489+
1490+
IndexSymbol info;
1491+
if (!reportRef(shadowedDecl, loc, info, AccessKind::Read))
1492+
return false;
1493+
1494+
// Suppress the reference if there is any (it is implicit and hence
1495+
// already skipped in the shorthand if let case, but explicit in the
1496+
// captured case).
1497+
repressRefAtLoc(loc);
1498+
1499+
// Skip the definition of a shadowed decl
1500+
return true;
1501+
}
1502+
14411503
if (startEntityDecl(D)) {
14421504
// Pass accessors.
14431505
if (auto StoreD = dyn_cast<AbstractStorageDecl>(D)) {

test/Index/index_shadow.swift

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s --include-locals | %FileCheck %s
3+
4+
// The index will output references to the shadowed-declaration rather than
5+
// the one defined by the shorthand if-let or capture. It also skips
6+
// outputting the shadowing-definiiton since it would then have no references.
7+
8+
struct ShadowedTest {
9+
// CHECK: [[@LINE+1]]:7 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Def
10+
let shadowedVar: Int?? = 1
11+
12+
func localShadowTest() {
13+
// CHECK: [[@LINE+1]]:9 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Def
14+
let localShadowedVar: Int? = 2
15+
16+
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} localShadowedVar {{.*}}Def
17+
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
18+
if let localShadowedVar {
19+
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
20+
_ = localShadowedVar
21+
}
22+
23+
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} localShadowedVar {{.*}}Def
24+
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
25+
_ = { [localShadowedVar] in
26+
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
27+
_ = localShadowedVar
28+
}
29+
}
30+
31+
func shadowTest() {
32+
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} shadowedVar {{.*}}Def
33+
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
34+
if let shadowedVar {
35+
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
36+
_ = shadowedVar
37+
38+
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedVar {{.*}}Def
39+
// CHECK: [[@LINE+1]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
40+
if let shadowedVar {
41+
// CHECK: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
42+
_ = shadowedVar
43+
}
44+
}
45+
46+
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} shadowedVar {{.*}}Def
47+
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
48+
_ = { [shadowedVar] in
49+
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
50+
_ = shadowedVar
51+
52+
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedVar {{.*}}Def
53+
// CHECK: [[@LINE+1]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
54+
_ = { [shadowedVar] in
55+
// CHECK: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
56+
_ = shadowedVar
57+
}
58+
}
59+
}
60+
}

0 commit comments

Comments
 (0)