Skip to content

Commit 4b4bbbc

Browse files
HighCommander4HighCommander4
committed
Address other review comments
1 parent 9a39394 commit 4b4bbbc

File tree

12 files changed

+76
-67
lines changed

12 files changed

+76
-67
lines changed

‎clang-tools-extra/clangd/XRefs.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,20 +2317,16 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
23172317
return Results;
23182318
}
23192319
// In this function, we find outgoing calls based on the index only.
2320-
RefsRequest Request;
2321-
Request.IDs.insert(*ID);
2322-
// Note that RefKind::Call just restricts the matched SymbolKind to
2323-
// functions, not the form of the reference (e.g. address-of-function,
2324-
// which can indicate an indirect call, should still be caught).
2325-
Request.Filter = RefKind::Call;
2320+
ContainedRefsRequest Request;
2321+
Request.ID = *ID;
23262322
// Initially store the ranges in a map keyed by SymbolID of the callee.
23272323
// This allows us to group different calls to the same function
23282324
// into the same CallHierarchyOutgoingCall.
23292325
llvm::DenseMap<SymbolID, std::vector<Range>> CallsOut;
23302326
// We can populate the ranges based on a refs request only. As we do so, we
23312327
// also accumulate the callee IDs into a lookup request.
23322328
LookupRequest CallsOutLookup;
2333-
Index->refersTo(Request, [&](const auto &R) {
2329+
Index->containedRefs(Request, [&](const auto &R) {
23342330
auto Loc = indexToLSPLocation(R.Location, Item.uri.file());
23352331
if (!Loc) {
23362332
elog("outgoingCalls failed to convert location: {0}", Loc.takeError());

‎clang-tools-extra/clangd/index/Index.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ bool SwapIndex::refs(const RefsRequest &R,
6666
llvm::function_ref<void(const Ref &)> CB) const {
6767
return snapshot()->refs(R, CB);
6868
}
69-
bool SwapIndex::refersTo(
70-
const RefsRequest &R,
71-
llvm::function_ref<void(const RefersToResult &)> CB) const {
72-
return snapshot()->refersTo(R, CB);
69+
bool SwapIndex::containedRefs(
70+
const ContainedRefsRequest &R,
71+
llvm::function_ref<void(const ContainedRefsResult &)> CB) const {
72+
return snapshot()->containedRefs(R, CB);
7373
}
7474
void SwapIndex::relations(
7575
const RelationsRequest &R,

‎clang-tools-extra/clangd/index/Index.h

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,27 @@ struct RefsRequest {
7777
bool WantContainer = false;
7878
};
7979

80+
struct ContainedRefsRequest {
81+
/// Note that RefKind::Call just restricts the matched SymbolKind to
82+
/// functions, not the form of the reference (e.g. address-of-function,
83+
/// which can indicate an indirect call, should still be caught).
84+
static const RefKind SupportedRefKinds = RefKind::Call;
85+
86+
SymbolID ID;
87+
/// If set, limit the number of refers returned from the index. The index may
88+
/// choose to return less than this, e.g. it tries to avoid returning stale
89+
/// results.
90+
std::optional<uint32_t> Limit;
91+
};
92+
8093
struct RelationsRequest {
8194
llvm::DenseSet<SymbolID> Subjects;
8295
RelationKind Predicate;
8396
/// If set, limit the number of relations returned from the index.
8497
std::optional<uint32_t> Limit;
8598
};
8699

87-
struct RefersToResult {
100+
struct ContainedRefsResult {
88101
/// The source location where the symbol is named.
89102
SymbolLocation Location;
90103
RefKind Kind = RefKind::Unknown;
@@ -156,9 +169,9 @@ class SymbolIndex {
156169
/// The returned result must be deep-copied if it's used outside Callback.
157170
///
158171
/// Returns true if there will be more results (limited by Req.Limit);
159-
virtual bool
160-
refersTo(const RefsRequest &Req,
161-
llvm::function_ref<void(const RefersToResult &)> Callback) const = 0;
172+
virtual bool containedRefs(
173+
const ContainedRefsRequest &Req,
174+
llvm::function_ref<void(const ContainedRefsResult &)> Callback) const = 0;
162175

163176
/// Finds all relations (S, P, O) stored in the index such that S is among
164177
/// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in
@@ -194,9 +207,9 @@ class SwapIndex : public SymbolIndex {
194207
llvm::function_ref<void(const Symbol &)>) const override;
195208
bool refs(const RefsRequest &,
196209
llvm::function_ref<void(const Ref &)>) const override;
197-
bool
198-
refersTo(const RefsRequest &,
199-
llvm::function_ref<void(const RefersToResult &)>) const override;
210+
bool containedRefs(
211+
const ContainedRefsRequest &,
212+
llvm::function_ref<void(const ContainedRefsResult &)>) const override;
200213
void relations(const RelationsRequest &,
201214
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
202215
const override;

‎clang-tools-extra/clangd/index/MemIndex.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "MemIndex.h"
1010
#include "FuzzyMatch.h"
1111
#include "Quality.h"
12+
#include "index/Index.h"
1213
#include "support/Trace.h"
1314

1415
namespace clang {
@@ -85,15 +86,15 @@ bool MemIndex::refs(const RefsRequest &Req,
8586
return false; // We reported all refs.
8687
}
8788

88-
bool MemIndex::refersTo(
89-
const RefsRequest &Req,
90-
llvm::function_ref<void(const RefersToResult &)> Callback) const {
89+
bool MemIndex::containedRefs(
90+
const ContainedRefsRequest &Req,
91+
llvm::function_ref<void(const ContainedRefsResult &)> Callback) const {
9192
trace::Span Tracer("MemIndex refersTo");
9293
uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
9394
for (const auto &Pair : Refs) {
9495
for (const auto &R : Pair.second) {
95-
if (!static_cast<int>(Req.Filter & R.Kind) ||
96-
!Req.IDs.contains(R.Container))
96+
if (!static_cast<int>(ContainedRefsRequest::SupportedRefKinds & R.Kind) ||
97+
Req.ID != R.Container)
9798
continue;
9899
if (Remaining == 0)
99100
return true; // More refs were available.

‎clang-tools-extra/clangd/index/MemIndex.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ class MemIndex : public SymbolIndex {
7272
bool refs(const RefsRequest &Req,
7373
llvm::function_ref<void(const Ref &)> Callback) const override;
7474

75-
bool refersTo(
76-
const RefsRequest &Req,
77-
llvm::function_ref<void(const RefersToResult &)> Callback) const override;
75+
bool containedRefs(const ContainedRefsRequest &Req,
76+
llvm::function_ref<void(const ContainedRefsResult &)>
77+
Callback) const override;
7878

7979
void relations(const RelationsRequest &Req,
8080
llvm::function_ref<void(const SymbolID &, const Symbol &)>

‎clang-tools-extra/clangd/index/Merge.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,17 @@ bool MergedIndex::refs(const RefsRequest &Req,
155155
return More || StaticHadMore;
156156
}
157157

158-
bool MergedIndex::refersTo(
159-
const RefsRequest &Req,
160-
llvm::function_ref<void(const RefersToResult &)> Callback) const {
158+
bool MergedIndex::containedRefs(
159+
const ContainedRefsRequest &Req,
160+
llvm::function_ref<void(const ContainedRefsResult &)> Callback) const {
161161
trace::Span Tracer("MergedIndex refersTo");
162162
bool More = false;
163163
uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
164164
// We don't want duplicated refs from the static/dynamic indexes,
165165
// and we can't reliably deduplicate them because offsets may differ slightly.
166166
// We consider the dynamic index authoritative and report all its refs,
167167
// and only report static index refs from other files.
168-
More |= Dynamic->refersTo(Req, [&](const auto &O) {
168+
More |= Dynamic->containedRefs(Req, [&](const auto &O) {
169169
Callback(O);
170170
assert(Remaining != 0);
171171
--Remaining;
@@ -175,7 +175,7 @@ bool MergedIndex::refersTo(
175175
auto DynamicContainsFile = Dynamic->indexedFiles();
176176
// We return less than Req.Limit if static index returns more refs for dirty
177177
// files.
178-
bool StaticHadMore = Static->refersTo(Req, [&](const auto &O) {
178+
bool StaticHadMore = Static->containedRefs(Req, [&](const auto &O) {
179179
if ((DynamicContainsFile(O.Location.FileURI) & IndexContents::References) !=
180180
IndexContents::None)
181181
return; // ignore refs that have been seen from dynamic index.

‎clang-tools-extra/clangd/index/Merge.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ class MergedIndex : public SymbolIndex {
3838
llvm::function_ref<void(const Symbol &)>) const override;
3939
bool refs(const RefsRequest &,
4040
llvm::function_ref<void(const Ref &)>) const override;
41-
bool
42-
refersTo(const RefsRequest &,
43-
llvm::function_ref<void(const RefersToResult &)>) const override;
41+
bool containedRefs(
42+
const ContainedRefsRequest &,
43+
llvm::function_ref<void(const ContainedRefsResult &)>) const override;
4444
void relations(const RelationsRequest &,
4545
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
4646
const override;

‎clang-tools-extra/clangd/index/ProjectAware.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ class ProjectAwareIndex : public SymbolIndex {
3636
bool refs(const RefsRequest &Req,
3737
llvm::function_ref<void(const Ref &)> Callback) const override;
3838
/// Query all indexes while prioritizing the associated one (if any).
39-
bool refersTo(
40-
const RefsRequest &Req,
41-
llvm::function_ref<void(const RefersToResult &)> Callback) const override;
39+
bool containedRefs(const ContainedRefsRequest &Req,
40+
llvm::function_ref<void(const ContainedRefsResult &)>
41+
Callback) const override;
4242

4343
/// Queries only the associates index when Req.RestrictForCodeCompletion is
4444
/// set, otherwise queries all.
@@ -98,12 +98,12 @@ bool ProjectAwareIndex::refs(
9898
return false;
9999
}
100100

101-
bool ProjectAwareIndex::refersTo(
102-
const RefsRequest &Req,
103-
llvm::function_ref<void(const RefersToResult &)> Callback) const {
101+
bool ProjectAwareIndex::containedRefs(
102+
const ContainedRefsRequest &Req,
103+
llvm::function_ref<void(const ContainedRefsResult &)> Callback) const {
104104
trace::Span Tracer("ProjectAwareIndex::refersTo");
105105
if (auto *Idx = getIndex())
106-
return Idx->refersTo(Req, Callback);
106+
return Idx->containedRefs(Req, Callback);
107107
return false;
108108
}
109109

‎clang-tools-extra/clangd/index/dex/Dex.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ void Dex::buildIndex() {
151151
// Build RevRefs
152152
for (const auto &[ID, RefList] : Refs)
153153
for (const auto &R : RefList)
154-
if ((R.Kind & RefKind::Call) != RefKind::Unknown)
154+
if ((R.Kind & ContainedRefsRequest::SupportedRefKinds) !=
155+
RefKind::Unknown)
155156
RevRefs.emplace_back(R, ID);
156157
// Sort by container ID so we can use binary search for lookup.
157158
llvm::sort(RevRefs, [](const RevRef &A, const RevRef &B) {
@@ -339,20 +340,18 @@ Dex::lookupRevRefs(const SymbolID &Container) const {
339340
return {ItPair.first, ItPair.second};
340341
}
341342

342-
bool Dex::refersTo(
343-
const RefsRequest &Req,
344-
llvm::function_ref<void(const RefersToResult &)> Callback) const {
343+
bool Dex::containedRefs(
344+
const ContainedRefsRequest &Req,
345+
llvm::function_ref<void(const ContainedRefsResult &)> Callback) const {
345346
trace::Span Tracer("Dex reversed refs");
346347
uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
347-
for (const auto &ID : Req.IDs)
348-
for (const auto &Rev : lookupRevRefs(ID)) {
349-
if (!static_cast<int>(Req.Filter & Rev.ref().Kind))
350-
continue;
351-
if (Remaining == 0)
352-
return true; // More refs were available.
353-
--Remaining;
354-
Callback(Rev.refersToResult());
355-
}
348+
for (const auto &Rev : lookupRevRefs(Req.ID)) {
349+
// RevRefs are already filtered to ContainedRefsRequest::SupportedRefKinds
350+
if (Remaining == 0)
351+
return true; // More refs were available.
352+
--Remaining;
353+
Callback(Rev.containedRefsResult());
354+
}
356355
return false; // We reported all refs.
357356
}
358357

‎clang-tools-extra/clangd/index/dex/Dex.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ class Dex : public SymbolIndex {
8585
bool refs(const RefsRequest &Req,
8686
llvm::function_ref<void(const Ref &)> Callback) const override;
8787

88-
bool refersTo(
89-
const RefsRequest &Req,
90-
llvm::function_ref<void(const RefersToResult &)> Callback) const override;
88+
bool containedRefs(const ContainedRefsRequest &Req,
89+
llvm::function_ref<void(const ContainedRefsResult &)>
90+
Callback) const override;
9191

9292
void relations(const RelationsRequest &Req,
9393
llvm::function_ref<void(const SymbolID &, const Symbol &)>
@@ -107,7 +107,7 @@ class Dex : public SymbolIndex {
107107
RevRef(const Ref &Reference, SymbolID Target)
108108
: Reference(&Reference), Target(Target) {}
109109
const Ref &ref() const { return *Reference; }
110-
RefersToResult refersToResult() const {
110+
ContainedRefsResult containedRefsResult() const {
111111
return {ref().Location, ref().Kind, Target};
112112
}
113113
};

‎clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,9 +1702,9 @@ class IndexRequestCollector : public SymbolIndex {
17021702
return false;
17031703
}
17041704

1705-
bool
1706-
refersTo(const RefsRequest &,
1707-
llvm::function_ref<void(const RefersToResult &)>) const override {
1705+
bool containedRefs(
1706+
const ContainedRefsRequest &,
1707+
llvm::function_ref<void(const ContainedRefsResult &)>) const override {
17081708
return false;
17091709
}
17101710

‎clang-tools-extra/clangd/unittests/RenameTests.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,9 +1601,9 @@ TEST(CrossFileRenameTests, DirtyBuffer) {
16011601
return true; // has more references
16021602
}
16031603

1604-
bool refersTo(const RefsRequest &Req,
1605-
llvm::function_ref<void(const RefersToResult &)> Callback)
1606-
const override {
1604+
bool containedRefs(const ContainedRefsRequest &Req,
1605+
llvm::function_ref<void(const ContainedRefsResult &)>
1606+
Callback) const override {
16071607
return false;
16081608
}
16091609

@@ -1658,9 +1658,9 @@ TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
16581658
return false;
16591659
}
16601660

1661-
bool refersTo(const RefsRequest &Req,
1662-
llvm::function_ref<void(const RefersToResult &)> Callback)
1663-
const override {
1661+
bool containedRefs(const ContainedRefsRequest &Req,
1662+
llvm::function_ref<void(const ContainedRefsResult &)>
1663+
Callback) const override {
16641664
return false;
16651665
}
16661666

0 commit comments

Comments
 (0)