Skip to content

Commit cd480a9

Browse files
hamishknighthamishknight
committed
[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
1 parent fb4ffb4 commit cd480a9

File tree

3 files changed

+110
-36
lines changed

3 files changed

+110
-36
lines changed

‎lib/Index/Index.cpp

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -216,26 +216,38 @@ 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;
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;
222225

223-
const StackEntry &Entry = Stack.back();
226+
auto MapEntry = Entry.Containers.find(Entry.ActiveKey);
227+
if (MapEntry == Entry.Containers.end())
228+
return;
224229

225-
if (!Entry.ActiveKey)
226-
return;
230+
SmallVector<const Decl *, 4> Containers;
231+
if (auto C = MapEntry->second) {
232+
if (auto *D = C.dyn_cast<const Decl *>()) {
233+
if (allowDecl(D))
234+
Containers.push_back(D);
235+
} else {
236+
auto *P = C.get<const Pattern *>();
237+
P->forEachVariable([&](VarDecl *VD) {
238+
if (allowDecl(VD))
239+
Containers.push_back(VD);
240+
});
241+
}
242+
}
243+
// If we didn't have any viable containers, continue walking up the stack.
244+
if (Containers.empty())
245+
continue;
227246

228-
auto MapEntry = Entry.Containers.find(Entry.ActiveKey);
247+
for (auto *decl : Containers)
248+
f(decl);
229249

230-
if (MapEntry == Entry.Containers.end())
231250
return;
232-
233-
Container C = MapEntry->second;
234-
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); });
239251
}
240252
}
241253

@@ -343,26 +355,10 @@ class ContainerTracker {
343355
}
344356

345357
// 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.
358+
// VarDecl. We store null here, and will walk up to the parent container in
359+
// forEachActiveContainer.
348360
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;
361+
Entry.Containers[K] = nullptr;
366362
}
367363

368364
void associateAllPatternElements(const Pattern *P, ActivationKey K,
@@ -924,7 +920,14 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
924920
}
925921

926922
void addContainedByRelationIfContained(IndexSymbol &Info) {
927-
Containers.forEachActiveContainer([&](const Decl *D) {
923+
// Only consider the innermost container that we are allowed to index.
924+
auto allowDecl = [&](const Decl *D) {
925+
if (auto *VD = dyn_cast<ValueDecl>(D)) {
926+
return shouldIndex(VD, /*IsRef*/ true);
927+
}
928+
return true;
929+
};
930+
Containers.forEachActiveContainer(allowDecl, [&](const Decl *D) {
928931
addRelation(Info, (unsigned)SymbolRole::RelationContainedBy,
929932
const_cast<Decl *>(D));
930933
});
@@ -1044,7 +1047,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
10441047
return {{line, col, inGeneratedBuffer}};
10451048
}
10461049

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

‎test/Index/roles-contained.swift

Lines changed: 41 additions & 0 deletions
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

Lines changed: 30 additions & 0 deletions
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)