From a02d45729263b700aa980ab3068199de048932bd Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 9 Apr 2024 21:28:22 +0100 Subject: [PATCH 1/2] [Index] Avoid reporting containers for non-indexed decls Check to see whether we can index the given decl before reporting it as a container, walking up to a parent if we need to. This also lets us simplify the AnyPattern handling a bit. rdar://126137541 --- lib/Index/Index.cpp | 80 ++++++++++++++++-------------- test/Index/property_wrappers.swift | 13 +++-- test/Index/roles-contained.swift | 41 +++++++++++++++ test/Index/roles.swift | 30 +++++++++++ 4 files changed, 122 insertions(+), 42 deletions(-) diff --git a/lib/Index/Index.cpp b/lib/Index/Index.cpp index 7f2eaccae9c52..22eebb4de3675 100644 --- a/lib/Index/Index.cpp +++ b/lib/Index/Index.cpp @@ -216,26 +216,39 @@ class ContainerTracker { bool empty() const { return Stack.empty(); } - void forEachActiveContainer(llvm::function_ref f) const { - if (Stack.empty()) - return; - - const StackEntry &Entry = Stack.back(); - - if (!Entry.ActiveKey) - return; - - auto MapEntry = Entry.Containers.find(Entry.ActiveKey); + void forEachActiveContainer(llvm::function_ref allowDecl, + llvm::function_ref f) const { + for (const auto &Entry : llvm::reverse(Stack)) { + // No active container, we're done. + if (!Entry.ActiveKey) + return; - if (MapEntry == Entry.Containers.end()) - return; + auto MapEntry = Entry.Containers.find(Entry.ActiveKey); + if (MapEntry == Entry.Containers.end()) + return; - Container C = MapEntry->second; + bool hadViableContainer = false; + auto tryContainer = [&](const Decl *D) { + if (!allowDecl(D)) + return; - if (auto *D = C.dyn_cast()) { - f(D); - } else if (auto *P = C.dyn_cast()) { - P->forEachVariable([&](VarDecl *VD) { f(VD); }); + f(D); + hadViableContainer = true; + }; + if (auto C = MapEntry->second) { + if (auto *D = C.dyn_cast()) { + tryContainer(D); + } else { + auto *P = C.get(); + P->forEachVariable([&](VarDecl *VD) { + tryContainer(VD); + }); + } + } + // If we had a viable containers, we're done. Otherwise continue walking + // up the stack. + if (hadViableContainer) + return; } } @@ -343,26 +356,10 @@ class ContainerTracker { } // AnyPatterns behave differently to other patterns as they've no associated - // VarDecl. The given ActivationKey is therefore associated with the current - // active container, if any. + // VarDecl. We store null here, and will walk up to the parent container in + // forEachActiveContainer. void associateAnyPattern(ActivationKey K, StackEntry &Entry) const { - Entry.Containers[K] = activeContainer(); - } - - Container activeContainer() const { - if (Stack.empty()) - return nullptr; - - const StackEntry &Entry = Stack.back(); - - if (Entry.ActiveKey) { - auto ActiveContainer = Entry.Containers.find(Entry.ActiveKey); - - if (ActiveContainer != Entry.Containers.end()) - return ActiveContainer->second; - } - - return nullptr; + Entry.Containers[K] = nullptr; } void associateAllPatternElements(const Pattern *P, ActivationKey K, @@ -924,7 +921,14 @@ class IndexSwiftASTWalker : public SourceEntityWalker { } void addContainedByRelationIfContained(IndexSymbol &Info) { - Containers.forEachActiveContainer([&](const Decl *D) { + // Only consider the innermost container that we are allowed to index. + auto allowDecl = [&](const Decl *D) { + if (auto *VD = dyn_cast(D)) { + return shouldIndex(VD, /*IsRef*/ true); + } + return true; + }; + Containers.forEachActiveContainer(allowDecl, [&](const Decl *D) { addRelation(Info, (unsigned)SymbolRole::RelationContainedBy, const_cast(D)); }); @@ -1044,7 +1048,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker { return {{line, col, inGeneratedBuffer}}; } - bool shouldIndex(ValueDecl *D, bool IsRef) const { + bool shouldIndex(const ValueDecl *D, bool IsRef) const { if (D->isImplicit() && isa(D) && IsRef) { // Bypass the implicit VarDecls introduced in CaseStmt bodies by using the // canonical VarDecl for these checks instead. diff --git a/test/Index/property_wrappers.swift b/test/Index/property_wrappers.swift index ca53d118edd39..0dc56f8f93290 100644 --- a/test/Index/property_wrappers.swift +++ b/test/Index/property_wrappers.swift @@ -46,18 +46,23 @@ public struct HasWrappers { // CHECK-NOT: [[@LINE-6]]:20 | variable/Swift | globalInt @Wrapper(body: { - // CHECK: [[@LINE-1]]:4 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1 + // CHECK: [[@LINE-1]]:4 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1 + // CHECK-NEXT: RelCont | instance-property/Swift | z struct Inner { @Wrapper - // CHECK: [[@LINE-1]]:8 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1 - // CHECK: [[@LINE-2]]:8 | constructor/Swift | init(initialValue:) | [[WrapperInit_USR]] | Ref,Call,Impl,RelCont | rel: 1 + // CHECK: [[@LINE-1]]:8 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1 + // CHECK-NEXT: RelCont | instance-property/Swift | z + // CHECK: [[@LINE-3]]:8 | constructor/Swift | init(initialValue:) | [[WrapperInit_USR]] | Ref,Call,Impl,RelCont | rel: 1 + // CHECK-NEXT: RelCont | instance-property/Swift | z var x: Int = globalInt // CHECK: [[@LINE-1]]:20 | variable/Swift | globalInt | [[globalInt_USR]] | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | instance-property/Swift | z } return Inner().x + globalInt // CHECK: [[@LINE-1]]:24 | variable/Swift | globalInt | [[globalInt_USR]] | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | instance-property/Swift | z }) - // CHECK: [[@LINE-12]]:4 | constructor/Swift | init(body:) | [[WrapperBodyInit_USR]] | Ref,Call,RelCont | rel: 1 + // CHECK: [[@LINE-17]]:4 | constructor/Swift | init(body:) | [[WrapperBodyInit_USR]] | Ref,Call,RelCont | rel: 1 public var z: Int // CHECK: [[@LINE-1]]:14 | instance-property/Swift | z | [[z_USR:.*]] | Def,RelChild | rel: 1 diff --git a/test/Index/roles-contained.swift b/test/Index/roles-contained.swift index 2256539ea01ed..d24d649135153 100644 --- a/test/Index/roles-contained.swift +++ b/test/Index/roles-contained.swift @@ -254,6 +254,47 @@ func containingFunc(param: Int) { // CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained | {{.*}} // CHECK: [[@LINE-11]]:78 | function/acc-get/Swift | getter:stringValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2 // CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained | {{.*}} + + let (_, tupleIgnoredSiblingElementContained): (Int, String) = ( + { let x = intValue; return x }(), + { let y = stringValue; return y }() + ) + // CHECK: [[@LINE-3]]:13 | variable(local)/Swift | x | {{.*}} | Def,RelChild | rel: 1 + // CHECK-NEXT: RelChild | function/Swift | containingFunc(param:) + + // CHECK: [[@LINE-6]]:17 | variable/Swift | intValue | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | variable(local)/Swift | x + + // Here the reference to intValue is contained by 'x'. + // CHECK: [[@LINE-10]]:17 | function/acc-get/Swift | getter:intValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2 + // CHECK-NEXT: RelCont | variable(local)/Swift | x + // CHECK-NEXT: RelCall | function/Swift | containingFunc(param:) + + // But here the container for the reference to 'x' is the parent function. + // CHECK: [[@LINE-15]]:34 | variable(local)/Swift | x | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | function/Swift | containingFunc(param:) + + // CHECK: [[@LINE-18]]:34 | function/acc-get(local)/Swift | getter:x | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1 + // CHECK-NEXT: RelCall,RelCont | function/Swift | containingFunc(param:) + + // CHECK: [[@LINE-20]]:13 | variable(local)/Swift | y | {{.*}} | Def,RelChild | rel: 1 + // CHECK-NEXT: RelChild | function/Swift | containingFunc(param:) + + // CHECK: [[@LINE-23]]:17 | variable/Swift | stringValue | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | variable(local)/Swift | y + + // Here the reference to stringValue is contained by 'y'. + // CHECK: [[@LINE-27]]:17 | function/acc-get/Swift | getter:stringValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2 + // CHECK-NEXT: RelCont | variable(local)/Swift | y + // CHECK-NEXT: RelCall | function/Swift | containingFunc(param:) + + // But here the container for the reference to 'y' is the parent binding. + // CHECK: [[@LINE-32]]:37 | variable(local)/Swift | y | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained + + // CHECK: [[@LINE-35]]:37 | function/acc-get(local)/Swift | getter:y | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2 + // CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained + // CHECK-NEXT: RelCall | function/Swift | containingFunc(param:) } func functionWithReturnType() -> Int { 0 } diff --git a/test/Index/roles.swift b/test/Index/roles.swift index 7fb62d09ec10e..9b48816d26a82 100644 --- a/test/Index/roles.swift +++ b/test/Index/roles.swift @@ -509,3 +509,33 @@ extension Alias { // CHECK: [[@LINE-2]]:11 | struct/Swift | Root | [[Root_USR]] | Ref,Impl,RelExt | rel: 1 func empty() {} } + +func returnsInt() -> Int { 0 } + +func containerFunc() { + // Make sure all the references here are contained by the function. + let i = returnsInt() + // CHECK: [[@LINE-1]]:11 | function/Swift | returnsInt() | {{.*}} | Ref,Call,RelCall,RelCont | rel: 1 + // CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc() + + let (_, k): (Int, Int) = ( + { let a = x; return a }(), + { let b = y; return b }() + ) + // CHECK: [[@LINE-4]]:16 | struct/Swift | Int | s:Si | Ref,RelCont | rel: 1 + // CHECK-NEXT: RelCont | function/Swift | containerFunc() + // CHECK: [[@LINE-6]]:21 | struct/Swift | Int | s:Si | Ref,RelCont | rel: 1 + // CHECK-NEXT: RelCont | function/Swift | containerFunc() + + // CHECK: [[@LINE-8]]:15 | variable/Swift | x | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | function/Swift | containerFunc() + + // CHECK: [[@LINE-11]]:15 | function/acc-get/Swift | getter:x | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1 + // CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc() + + // CHECK: [[@LINE-13]]:15 | variable/Swift | y | {{.*}} | Ref,Read,RelCont | rel: 1 + // CHECK-NEXT: RelCont | function/Swift | containerFunc() + + // CHECK: [[@LINE-16]]:15 | function/acc-get/Swift | getter:y | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1 + // CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc() +} From f2eb68c867f46e25725b34718c29e17478962ee6 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 9 Apr 2024 21:28:22 +0100 Subject: [PATCH 2/2] [Index] Avoid forming relations to non-indexed decls If we shouldn't index the related decl, don't record it as a relation. --- lib/Index/Index.cpp | 4 ++++ test/Index/index_module_inheritance_access.swift | 15 +++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 test/Index/index_module_inheritance_access.swift diff --git a/lib/Index/Index.cpp b/lib/Index/Index.cpp index 22eebb4de3675..80113399acf5b 100644 --- a/lib/Index/Index.cpp +++ b/lib/Index/Index.cpp @@ -544,6 +544,10 @@ class IndexSwiftASTWalker : public SourceEntityWalker { bool addRelation(IndexSymbol &Info, SymbolRoleSet RelationRoles, Decl *D) { assert(D); + if (auto *VD = dyn_cast(D)) { + if (!shouldIndex(VD, /*IsRef*/ true)) + return true; + } auto Match = std::find_if(Info.Relations.begin(), Info.Relations.end(), [D](IndexRelation R) { return R.decl == D; }); if (Match != Info.Relations.end()) { diff --git a/test/Index/index_module_inheritance_access.swift b/test/Index/index_module_inheritance_access.swift new file mode 100644 index 0000000000000..3db4fed5c66fd --- /dev/null +++ b/test/Index/index_module_inheritance_access.swift @@ -0,0 +1,15 @@ +// RUN: %empty-directory(%t) +// +// RUN: %target-swift-frontend -emit-module -emit-module-path %t/Mod.swiftmodule -module-name Mod %s +// RUN: %target-swift-ide-test -print-indexed-symbols -module-to-print Mod -source-filename %s -I %t | %FileCheck %s + +public class C { + fileprivate func foo() {} +} +public class D: C { + public override func foo() {} +} + +// Make sure we don't report the override of the private member in the base class. +//CHECK: instance-method/Swift | foo() | s:3Mod1DC3fooyyF | Def,Dyn,RelChild | rel: 1 +//CHECK-NEXT: RelChild | class/Swift | D | s:3Mod1DC