Skip to content

Emit references to shadowed variables in if-let shorthands #77177

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.

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 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions include/swift/Index/IndexSymbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ struct IndexSymbol : IndexRelation {
SmallVector<IndexRelation, 3> Relations;
unsigned line = 0;
unsigned column = 0;
const Decl *originalDecl = nullptr;

IndexSymbol() = default;

Expand Down
22 changes: 13 additions & 9 deletions lib/Index/Index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,13 +850,13 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
if (Loc.isInvalid() || isSuppressed(Loc))
return true;

IndexSymbol Info;

// Dig back to the original captured variable
if (auto *VD = dyn_cast<VarDecl>(D)) {
D = firstDecl(D);
Info.originalDecl = firstDecl(D);
}

IndexSymbol Info;

if (Data.isImplicit)
Info.roles |= (unsigned)SymbolRole::Implicit;

Expand Down Expand Up @@ -1580,13 +1580,17 @@ bool IndexSwiftASTWalker::report(ValueDecl *D) {
if (!reportRef(shadowedDecl, loc, info, AccessKind::Read))
return false;

// Suppress the reference if there is any (it is implicit and hence
// already skipped in the shorthand if let case, but explicit in the
// captured case).
suppressRefAtLoc(loc);
// Keep the refs and definition for the real decl when indexing locals,
// so the references to the shadowing variable are distinct.
if (!IdxConsumer.indexLocals()) {
// Suppress the reference if there is any (it is implicit and hence
// already skipped in the shorthand if let case, but explicit in the
// captured case).
suppressRefAtLoc(loc);

// Skip the definition of a shadowed decl
return true;
// Skip the definition of a shadowed decl
return true;
}
}

if (startEntityDecl(D)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Refactoring/LocalRename.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class RenameRangeCollector : public IndexDataConsumer {
bool finishDependency(bool isClangModule) override { return true; }

Action startSourceEntity(const IndexSymbol &symbol) override {
if (symbol.decl != declToRename) {
if (symbol.decl != declToRename && symbol.originalDecl != declToRename) {
return IndexDataConsumer::Continue;
}
auto loc = indexSymbolToRenameLoc(symbol);
Expand Down
Binary file added test.o
Binary file not shown.
50 changes: 34 additions & 16 deletions test/Index/index_shadow.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s --include-locals | %FileCheck %s
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck %s

// RUN: %empty-directory(%t)
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s -include-locals | %FileCheck %s -check-prefix=CHECK_LOCALS

// The index will output references to the shadowed-declaration rather than
// the one defined by the shorthand if-let or capture. It also skips
Expand All @@ -9,61 +12,76 @@ struct ShadowedTest {
let shadowedVar: Int?? = 1

func localShadowTest() {
// CHECK: [[@LINE+1]]:9 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Def
// CHECK_LOCALS: [[@LINE+1]]:9 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Def
let localShadowedVar: Int? = 2

// CHECK-NOT: [[@LINE+2]]:12 {{.*}} localShadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+1]]:12 | variable(local)/Swift{{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL0_Sivp {{.*}}Def
if let localShadowedVar {
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS-NOT: [[@LINE+2]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}
// CHECK_LOCALS: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL0_Sivp {{.*}}Ref
_ = localShadowedVar
}

// CHECK-NOT: [[@LINE+2]]:12 {{.*}} localShadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+1]]:12 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL1_SiSgvp {{.*}}Def
_ = { [localShadowedVar] in
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}Ref
// CHECK_LOCALS-NOT: [[@LINE+2]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL_SiSgvp {{.*}}
// CHECK_LOCALS: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV011localShadowE0yyF0fD3VarL1_SiSgvp {{.*}}Ref
_ = localShadowedVar
}
}

func shadowTest() {
// CHECK_LOCALS: [[@LINE+4]]:12 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL_SiSgvp {{.*}}Def
// CHECK_LOCALS: [[@LINE+3]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} shadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
if let shadowedVar {
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS-NOT: [[@LINE+3]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL_SiSgvp {{.*}}Ref
// CHECK-NOT: [[@LINE+1]]:11 {{.*}} shadowedVar {{.*}}Ref
_ = shadowedVar

// CHECK_LOCALS: [[@LINE+4]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+3]]:14 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL0_Sivp {{.*}}Def
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
if let shadowedVar {
// CHECK: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL0_Sivp {{.*}}Ref
// CHECK-NOT: [[@LINE+1]]:13 {{.*}} shadowedVar {{.*}}Ref
_ = shadowedVar
}
}

// CHECK_LOCALS: [[@LINE+4]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+3]]:12 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL1_SiSgSgvp {{.*}}Def
// CHECK-NOT: [[@LINE+2]]:12 {{.*}} shadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:12 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
_ = { [shadowedVar] in
// CHECK: [[@LINE+1]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:11 {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyF11shadowedVarL1_SiSgSgvp {{.*}}Ref
// CHECK-NOT: [[@LINE+1]]:11 {{.*}} shadowedVar {{.*}}Ref
_ = shadowedVar

// CHECK_LOCALS: [[@LINE+4]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+3]]:14 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyFyycfU_11shadowedVarL_SiSgSgvp {{.*}}Def
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedVar {{.*}}Def
// CHECK: [[@LINE+1]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
_ = { [shadowedVar] in
// CHECK: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV11shadowedVarSiSgSgvp {{.*}}Ref
// CHECK_LOCALS: [[@LINE+2]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV06shadowE0yyFyycfU_11shadowedVarL_SiSgSgvp {{.*}}Ref
// CHECK-NOT: [[@LINE+1]]:13 {{.*}} shadowedVar {{.*}}Ref
_ = shadowedVar
}
}
}

func nestedFuncTest() {
// CHECK: [[@LINE+1]]:10 {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyF {{.*}}Def
// CHECK_LOCALS: [[@LINE+1]]:10 {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyF {{.*}}Def
func shadowedFunc() {
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedFunc {{.*}}Def
// CHECK: [[@LINE+1]]:14 {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyF {{.*}}Ref
// CHECK_LOCALS: [[@LINE+1]]:14 | variable(local)/Swift {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyFAEL_yycvp {{.*}}Def
_ = { [shadowedFunc] in
// CHECK: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyF {{.*}}Ref
// CHECK-NOT: [[@LINE+2]]:14 {{.*}} shadowedFunc {{.*}}Def
// CHECK_LOCALS: [[@LINE+1]]:13 {{.*}} s:14swift_ide_test12ShadowedTestV010nestedFuncE0yyF08shadowedG0L_yyFAEL_yycvp {{.*}}Ref
_ = shadowedFunc
}
}
Expand Down