Skip to content

Commit f933f2a

Browse files
hamishknighthamishknight
authored
Merge pull request #72930 from hamishknight/containment
[Index] Avoid forming relations to non-indexed decls
2 parents 7fbfa59 + b65d8c2 commit f933f2a

File tree

5 files changed

+141
-42
lines changed

5 files changed

+141
-42
lines changed

‎lib/Index/Index.cpp

+46-38
Original file line numberDiff line numberDiff line change
@@ -216,26 +216,39 @@ class ContainerTracker {
216216

217217
bool empty() const { return Stack.empty(); }
218218

219-
void forEachActiveContainer(llvm::function_ref<void(const Decl *)> f) const {
220-
if (Stack.empty())
221-
return;
222-
223-
const StackEntry &Entry = Stack.back();
224-
225-
if (!Entry.ActiveKey)
226-
return;
227-
228-
auto MapEntry = Entry.Containers.find(Entry.ActiveKey);
219+
void forEachActiveContainer(llvm::function_ref<bool(const Decl *)> allowDecl,
220+
llvm::function_ref<void(const Decl *)> f) const {
221+
for (const auto &Entry : llvm::reverse(Stack)) {
222+
// No active container, we're done.
223+
if (!Entry.ActiveKey)
224+
return;
229225

230-
if (MapEntry == Entry.Containers.end())
231-
return;
226+
auto MapEntry = Entry.Containers.find(Entry.ActiveKey);
227+
if (MapEntry == Entry.Containers.end())
228+
return;
232229

233-
Container C = MapEntry->second;
230+
bool hadViableContainer = false;
231+
auto tryContainer = [&](const Decl *D) {
232+
if (!allowDecl(D))
233+
return;
234234

235-
if (auto *D = C.dyn_cast<const Decl *>()) {
236-
f(D);
237-
} else if (auto *P = C.dyn_cast<const Pattern *>()) {
238-
P->forEachVariable([&](VarDecl *VD) { f(VD); });
235+
f(D);
236+
hadViableContainer = true;
237+
};
238+
if (auto C = MapEntry->second) {
239+
if (auto *D = C.dyn_cast<const Decl *>()) {
240+
tryContainer(D);
241+
} else {
242+
auto *P = C.get<const Pattern *>();
243+
P->forEachVariable([&](VarDecl *VD) {
244+
tryContainer(VD);
245+
});
246+
}
247+
}
248+
// If we had a viable containers, we're done. Otherwise continue walking
249+
// up the stack.
250+
if (hadViableContainer)
251+
return;
239252
}
240253
}
241254

@@ -343,26 +356,10 @@ class ContainerTracker {
343356
}
344357

345358
// AnyPatterns behave differently to other patterns as they've no associated
346-
// VarDecl. The given ActivationKey is therefore associated with the current
347-
// active container, if any.
359+
// VarDecl. We store null here, and will walk up to the parent container in
360+
// forEachActiveContainer.
348361
void associateAnyPattern(ActivationKey K, StackEntry &Entry) const {
349-
Entry.Containers[K] = activeContainer();
350-
}
351-
352-
Container activeContainer() const {
353-
if (Stack.empty())
354-
return nullptr;
355-
356-
const StackEntry &Entry = Stack.back();
357-
358-
if (Entry.ActiveKey) {
359-
auto ActiveContainer = Entry.Containers.find(Entry.ActiveKey);
360-
361-
if (ActiveContainer != Entry.Containers.end())
362-
return ActiveContainer->second;
363-
}
364-
365-
return nullptr;
362+
Entry.Containers[K] = nullptr;
366363
}
367364

368365
void associateAllPatternElements(const Pattern *P, ActivationKey K,
@@ -547,6 +544,10 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
547544

548545
bool addRelation(IndexSymbol &Info, SymbolRoleSet RelationRoles, Decl *D) {
549546
assert(D);
547+
if (auto *VD = dyn_cast<ValueDecl>(D)) {
548+
if (!shouldIndex(VD, /*IsRef*/ true))
549+
return true;
550+
}
550551
auto Match = std::find_if(Info.Relations.begin(), Info.Relations.end(),
551552
[D](IndexRelation R) { return R.decl == D; });
552553
if (Match != Info.Relations.end()) {
@@ -924,7 +925,14 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
924925
}
925926

926927
void addContainedByRelationIfContained(IndexSymbol &Info) {
927-
Containers.forEachActiveContainer([&](const Decl *D) {
928+
// Only consider the innermost container that we are allowed to index.
929+
auto allowDecl = [&](const Decl *D) {
930+
if (auto *VD = dyn_cast<ValueDecl>(D)) {
931+
return shouldIndex(VD, /*IsRef*/ true);
932+
}
933+
return true;
934+
};
935+
Containers.forEachActiveContainer(allowDecl, [&](const Decl *D) {
928936
addRelation(Info, (unsigned)SymbolRole::RelationContainedBy,
929937
const_cast<Decl *>(D));
930938
});
@@ -1044,7 +1052,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
10441052
return {{line, col, inGeneratedBuffer}};
10451053
}
10461054

1047-
bool shouldIndex(ValueDecl *D, bool IsRef) const {
1055+
bool shouldIndex(const ValueDecl *D, bool IsRef) const {
10481056
if (D->isImplicit() && isa<VarDecl>(D) && IsRef) {
10491057
// Bypass the implicit VarDecls introduced in CaseStmt bodies by using the
10501058
// canonical VarDecl for these checks instead.

‎test/Index/index_module_inheritance_access.swift

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %empty-directory(%t)
2+
//
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/Mod.swiftmodule -module-name Mod %s
4+
// RUN: %target-swift-ide-test -print-indexed-symbols -module-to-print Mod -source-filename %s -I %t | %FileCheck %s
5+
6+
public class C {
7+
fileprivate func foo() {}
8+
}
9+
public class D: C {
10+
public override func foo() {}
11+
}
12+
13+
// Make sure we don't report the override of the private member in the base class.
14+
//CHECK: instance-method/Swift | foo() | s:3Mod1DC3fooyyF | Def,Dyn,RelChild | rel: 1
15+
//CHECK-NEXT: RelChild | class/Swift | D | s:3Mod1DC

‎test/Index/property_wrappers.swift

+9-4
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,23 @@ public struct HasWrappers {
4646
// CHECK-NOT: [[@LINE-6]]:20 | variable/Swift | globalInt
4747

4848
@Wrapper(body: {
49-
// CHECK: [[@LINE-1]]:4 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1
49+
// CHECK: [[@LINE-1]]:4 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1
50+
// CHECK-NEXT: RelCont | instance-property/Swift | z
5051
struct Inner {
5152
@Wrapper
52-
// CHECK: [[@LINE-1]]:8 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1
53-
// CHECK: [[@LINE-2]]:8 | constructor/Swift | init(initialValue:) | [[WrapperInit_USR]] | Ref,Call,Impl,RelCont | rel: 1
53+
// CHECK: [[@LINE-1]]:8 | struct/Swift | Wrapper | [[Wrapper_USR]] | Ref,RelCont | rel: 1
54+
// CHECK-NEXT: RelCont | instance-property/Swift | z
55+
// CHECK: [[@LINE-3]]:8 | constructor/Swift | init(initialValue:) | [[WrapperInit_USR]] | Ref,Call,Impl,RelCont | rel: 1
56+
// CHECK-NEXT: RelCont | instance-property/Swift | z
5457
var x: Int = globalInt
5558
// CHECK: [[@LINE-1]]:20 | variable/Swift | globalInt | [[globalInt_USR]] | Ref,Read,RelCont | rel: 1
59+
// CHECK-NEXT: RelCont | instance-property/Swift | z
5660
}
5761
return Inner().x + globalInt
5862
// CHECK: [[@LINE-1]]:24 | variable/Swift | globalInt | [[globalInt_USR]] | Ref,Read,RelCont | rel: 1
63+
// CHECK-NEXT: RelCont | instance-property/Swift | z
5964
})
60-
// CHECK: [[@LINE-12]]:4 | constructor/Swift | init(body:) | [[WrapperBodyInit_USR]] | Ref,Call,RelCont | rel: 1
65+
// CHECK: [[@LINE-17]]:4 | constructor/Swift | init(body:) | [[WrapperBodyInit_USR]] | Ref,Call,RelCont | rel: 1
6166
public var z: Int
6267
// CHECK: [[@LINE-1]]:14 | instance-property/Swift | z | [[z_USR:.*]] | Def,RelChild | rel: 1
6368

‎test/Index/roles-contained.swift

+41
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,47 @@ func containingFunc(param: Int) {
254254
// CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained | {{.*}}
255255
// CHECK: [[@LINE-11]]:78 | function/acc-get/Swift | getter:stringValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2
256256
// CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained | {{.*}}
257+
258+
let (_, tupleIgnoredSiblingElementContained): (Int, String) = (
259+
{ let x = intValue; return x }(),
260+
{ let y = stringValue; return y }()
261+
)
262+
// CHECK: [[@LINE-3]]:13 | variable(local)/Swift | x | {{.*}} | Def,RelChild | rel: 1
263+
// CHECK-NEXT: RelChild | function/Swift | containingFunc(param:)
264+
265+
// CHECK: [[@LINE-6]]:17 | variable/Swift | intValue | {{.*}} | Ref,Read,RelCont | rel: 1
266+
// CHECK-NEXT: RelCont | variable(local)/Swift | x
267+
268+
// Here the reference to intValue is contained by 'x'.
269+
// CHECK: [[@LINE-10]]:17 | function/acc-get/Swift | getter:intValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2
270+
// CHECK-NEXT: RelCont | variable(local)/Swift | x
271+
// CHECK-NEXT: RelCall | function/Swift | containingFunc(param:)
272+
273+
// But here the container for the reference to 'x' is the parent function.
274+
// CHECK: [[@LINE-15]]:34 | variable(local)/Swift | x | {{.*}} | Ref,Read,RelCont | rel: 1
275+
// CHECK-NEXT: RelCont | function/Swift | containingFunc(param:)
276+
277+
// CHECK: [[@LINE-18]]:34 | function/acc-get(local)/Swift | getter:x | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1
278+
// CHECK-NEXT: RelCall,RelCont | function/Swift | containingFunc(param:)
279+
280+
// CHECK: [[@LINE-20]]:13 | variable(local)/Swift | y | {{.*}} | Def,RelChild | rel: 1
281+
// CHECK-NEXT: RelChild | function/Swift | containingFunc(param:)
282+
283+
// CHECK: [[@LINE-23]]:17 | variable/Swift | stringValue | {{.*}} | Ref,Read,RelCont | rel: 1
284+
// CHECK-NEXT: RelCont | variable(local)/Swift | y
285+
286+
// Here the reference to stringValue is contained by 'y'.
287+
// CHECK: [[@LINE-27]]:17 | function/acc-get/Swift | getter:stringValue | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2
288+
// CHECK-NEXT: RelCont | variable(local)/Swift | y
289+
// CHECK-NEXT: RelCall | function/Swift | containingFunc(param:)
290+
291+
// But here the container for the reference to 'y' is the parent binding.
292+
// CHECK: [[@LINE-32]]:37 | variable(local)/Swift | y | {{.*}} | Ref,Read,RelCont | rel: 1
293+
// CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained
294+
295+
// CHECK: [[@LINE-35]]:37 | function/acc-get(local)/Swift | getter:y | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 2
296+
// CHECK-NEXT: RelCont | variable(local)/Swift | tupleIgnoredSiblingElementContained
297+
// CHECK-NEXT: RelCall | function/Swift | containingFunc(param:)
257298
}
258299

259300
func functionWithReturnType() -> Int { 0 }

‎test/Index/roles.swift

+30
Original file line numberDiff line numberDiff line change
@@ -509,3 +509,33 @@ extension Alias {
509509
// CHECK: [[@LINE-2]]:11 | struct/Swift | Root | [[Root_USR]] | Ref,Impl,RelExt | rel: 1
510510
func empty() {}
511511
}
512+
513+
func returnsInt() -> Int { 0 }
514+
515+
func containerFunc() {
516+
// Make sure all the references here are contained by the function.
517+
let i = returnsInt()
518+
// CHECK: [[@LINE-1]]:11 | function/Swift | returnsInt() | {{.*}} | Ref,Call,RelCall,RelCont | rel: 1
519+
// CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc()
520+
521+
let (_, k): (Int, Int) = (
522+
{ let a = x; return a }(),
523+
{ let b = y; return b }()
524+
)
525+
// CHECK: [[@LINE-4]]:16 | struct/Swift | Int | s:Si | Ref,RelCont | rel: 1
526+
// CHECK-NEXT: RelCont | function/Swift | containerFunc()
527+
// CHECK: [[@LINE-6]]:21 | struct/Swift | Int | s:Si | Ref,RelCont | rel: 1
528+
// CHECK-NEXT: RelCont | function/Swift | containerFunc()
529+
530+
// CHECK: [[@LINE-8]]:15 | variable/Swift | x | {{.*}} | Ref,Read,RelCont | rel: 1
531+
// CHECK-NEXT: RelCont | function/Swift | containerFunc()
532+
533+
// CHECK: [[@LINE-11]]:15 | function/acc-get/Swift | getter:x | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1
534+
// CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc()
535+
536+
// CHECK: [[@LINE-13]]:15 | variable/Swift | y | {{.*}} | Ref,Read,RelCont | rel: 1
537+
// CHECK-NEXT: RelCont | function/Swift | containerFunc()
538+
539+
// CHECK: [[@LINE-16]]:15 | function/acc-get/Swift | getter:y | {{.*}} | Ref,Call,Impl,RelCall,RelCont | rel: 1
540+
// CHECK-NEXT: RelCall,RelCont | function/Swift | containerFunc()
541+
}

0 commit comments

Comments
 (0)