Skip to content

Commit 1093b9f

Browse files
committed
Revert "[clangd] Elide even more checks in SelectionTree."
This reverts commit 07f9fb8.
1 parent 40f5f3d commit 1093b9f

File tree

2 files changed

+18
-102
lines changed

2 files changed

+18
-102
lines changed

clang-tools-extra/clangd/Selection.cpp

Lines changed: 18 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -274,37 +274,22 @@ class SelectionTester {
274274
for (unsigned I = 0; I < Sel.size(); ++I) {
275275
if (shouldIgnore(Sel[I]) || PPIgnored[I])
276276
continue;
277-
SelectedSpelled.emplace_back();
278-
Tok &S = SelectedSpelled.back();
277+
SpelledTokens.emplace_back();
278+
Tok &S = SpelledTokens.back();
279279
S.Offset = SM.getFileOffset(Sel[I].location());
280280
if (S.Offset >= SelBegin && S.Offset + Sel[I].length() <= SelEnd)
281281
S.Selected = SelectionTree::Complete;
282282
else
283283
S.Selected = SelectionTree::Partial;
284284
}
285-
MaybeSelectedExpanded = computeMaybeSelectedExpandedTokens(Buf);
286285
}
287286

288287
// Test whether a consecutive range of tokens is selected.
289288
// The tokens are taken from the expanded token stream.
290289
SelectionTree::Selection
291290
test(llvm::ArrayRef<syntax::Token> ExpandedTokens) const {
292-
if (ExpandedTokens.empty())
291+
if (SpelledTokens.empty())
293292
return NoTokens;
294-
if (SelectedSpelled.empty())
295-
return SelectionTree::Unselected;
296-
// Cheap (pointer) check whether any of the tokens could touch selection.
297-
// In most cases, the node's overall source range touches ExpandedTokens,
298-
// or we would have failed mayHit(). However now we're only considering
299-
// the *unclaimed* spans of expanded tokens.
300-
// This is a significant performance improvement when a lot of nodes
301-
// surround the selection, including when generated by macros.
302-
if (MaybeSelectedExpanded.empty() ||
303-
&ExpandedTokens.front() > &MaybeSelectedExpanded.back() ||
304-
&ExpandedTokens.back() < &MaybeSelectedExpanded.front()) {
305-
return SelectionTree::Unselected;
306-
}
307-
308293
SelectionTree::Selection Result = NoTokens;
309294
while (!ExpandedTokens.empty()) {
310295
// Take consecutive tokens from the same context together for efficiency.
@@ -327,14 +312,14 @@ class SelectionTester {
327312
// If it returns false, test() will return NoTokens or Unselected.
328313
// If it returns true, test() may return any value.
329314
bool mayHit(SourceRange R) const {
330-
if (SelectedSpelled.empty() || MaybeSelectedExpanded.empty())
315+
if (SpelledTokens.empty())
331316
return false;
332317
// If the node starts after the selection ends, it is not selected.
333318
// Tokens a macro location might claim are >= its expansion start.
334319
// So if the expansion start > last selected token, we can prune it.
335320
// (This is particularly helpful for GTest's TEST macro).
336321
if (auto B = offsetInSelFile(getExpansionStart(R.getBegin())))
337-
if (*B > SelectedSpelled.back().Offset)
322+
if (*B > SpelledTokens.back().Offset)
338323
return false;
339324
// If the node ends before the selection begins, it is not selected.
340325
SourceLocation EndLoc = R.getEnd();
@@ -343,72 +328,12 @@ class SelectionTester {
343328
// In the rare case that the expansion range is a char range, EndLoc is
344329
// ~one token too far to the right. We may fail to prune, that's OK.
345330
if (auto E = offsetInSelFile(EndLoc))
346-
if (*E < SelectedSpelled.front().Offset)
331+
if (*E < SpelledTokens.front().Offset)
347332
return false;
348333
return true;
349334
}
350335

351336
private:
352-
// Plausible expanded tokens that might be affected by the selection.
353-
// This is an overestimate, it may contain tokens that are not selected.
354-
// The point is to allow cheap pruning in test()
355-
llvm::ArrayRef<syntax::Token>
356-
computeMaybeSelectedExpandedTokens(const syntax::TokenBuffer &Toks) {
357-
if (SelectedSpelled.empty())
358-
return {};
359-
360-
bool StartInvalid = false;
361-
const syntax::Token *Start = llvm::partition_point(
362-
Toks.expandedTokens(),
363-
[&, First = SelectedSpelled.front().Offset](const syntax::Token &Tok) {
364-
if (Tok.kind() == tok::eof)
365-
return false;
366-
// Implausible if upperbound(Tok) < First.
367-
SourceLocation Loc = Tok.location();
368-
auto Offset = offsetInSelFile(Loc);
369-
while (Loc.isValid() && !Offset) {
370-
Loc = Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getEnd()
371-
: SM.getIncludeLoc(SM.getFileID(Loc));
372-
Offset = offsetInSelFile(Loc);
373-
}
374-
if (Offset)
375-
return *Offset < First;
376-
StartInvalid = true;
377-
return false; // conservatively assume this token can overlap
378-
});
379-
if (StartInvalid) {
380-
assert(false && "Expanded tokens could not be resolved to main file!");
381-
Start = Toks.expandedTokens().begin();
382-
}
383-
384-
bool EndInvalid = false;
385-
const syntax::Token *End = llvm::partition_point(
386-
Toks.expandedTokens(),
387-
[&, Last = SelectedSpelled.back().Offset](const syntax::Token &Tok) {
388-
if (Tok.kind() == tok::eof)
389-
return false;
390-
// Plausible if lowerbound(Tok) <= Last.
391-
SourceLocation Loc = Tok.location();
392-
auto Offset = offsetInSelFile(Loc);
393-
while (Loc.isValid() && !Offset) {
394-
Loc = Loc.isMacroID()
395-
? SM.getImmediateExpansionRange(Loc).getBegin()
396-
: SM.getIncludeLoc(SM.getFileID(Loc));
397-
Offset = offsetInSelFile(Loc);
398-
}
399-
if (Offset)
400-
return *Offset <= Last;
401-
EndInvalid = true;
402-
return true; // conservatively assume this token can overlap
403-
});
404-
if (EndInvalid) {
405-
assert(false && "Expanded tokens could not be resolved to main file!");
406-
End = Toks.expandedTokens().end();
407-
}
408-
409-
return llvm::makeArrayRef(Start, End);
410-
}
411-
412337
// Hit-test a consecutive range of tokens from a single file ID.
413338
SelectionTree::Selection
414339
testChunk(FileID FID, llvm::ArrayRef<syntax::Token> Batch) const {
@@ -464,20 +389,19 @@ class SelectionTester {
464389
SelectionTree::Selection testTokenRange(unsigned Begin, unsigned End) const {
465390
assert(Begin <= End);
466391
// Outside the selection entirely?
467-
if (End < SelectedSpelled.front().Offset ||
468-
Begin > SelectedSpelled.back().Offset)
392+
if (End < SpelledTokens.front().Offset ||
393+
Begin > SpelledTokens.back().Offset)
469394
return SelectionTree::Unselected;
470395

471396
// Compute range of tokens.
472397
auto B = llvm::partition_point(
473-
SelectedSpelled, [&](const Tok &T) { return T.Offset < Begin; });
474-
auto E = std::partition_point(B, SelectedSpelled.end(), [&](const Tok &T) {
475-
return T.Offset <= End;
476-
});
398+
SpelledTokens, [&](const Tok &T) { return T.Offset < Begin; });
399+
auto E = std::partition_point(
400+
B, SpelledTokens.end(), [&](const Tok &T) { return T.Offset <= End; });
477401

478402
// Aggregate selectedness of tokens in range.
479-
bool ExtendsOutsideSelection = Begin < SelectedSpelled.front().Offset ||
480-
End > SelectedSpelled.back().Offset;
403+
bool ExtendsOutsideSelection = Begin < SpelledTokens.front().Offset ||
404+
End > SpelledTokens.back().Offset;
481405
SelectionTree::Selection Result =
482406
ExtendsOutsideSelection ? SelectionTree::Unselected : NoTokens;
483407
for (auto It = B; It != E; ++It)
@@ -488,13 +412,13 @@ class SelectionTester {
488412
// Is the token at `Offset` selected?
489413
SelectionTree::Selection testToken(unsigned Offset) const {
490414
// Outside the selection entirely?
491-
if (Offset < SelectedSpelled.front().Offset ||
492-
Offset > SelectedSpelled.back().Offset)
415+
if (Offset < SpelledTokens.front().Offset ||
416+
Offset > SpelledTokens.back().Offset)
493417
return SelectionTree::Unselected;
494418
// Find the token, if it exists.
495419
auto It = llvm::partition_point(
496-
SelectedSpelled, [&](const Tok &T) { return T.Offset < Offset; });
497-
if (It != SelectedSpelled.end() && It->Offset == Offset)
420+
SpelledTokens, [&](const Tok &T) { return T.Offset < Offset; });
421+
if (It != SpelledTokens.end() && It->Offset == Offset)
498422
return It->Selected;
499423
return NoTokens;
500424
}
@@ -520,8 +444,7 @@ class SelectionTester {
520444
unsigned Offset;
521445
SelectionTree::Selection Selected;
522446
};
523-
std::vector<Tok> SelectedSpelled;
524-
llvm::ArrayRef<syntax::Token> MaybeSelectedExpanded;
447+
std::vector<Tok> SpelledTokens;
525448
FileID SelFile;
526449
SourceRange SelFileBounds;
527450
const SourceManager &SM;

clang-tools-extra/clangd/unittests/SelectionTests.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,6 @@ TEST(SelectionTest, CommonAncestor) {
201201
)cpp",
202202
nullptr,
203203
},
204-
{
205-
R"cpp(
206-
#define TARGET void foo()
207-
[[TAR^GET{ return; }]]
208-
)cpp",
209-
"FunctionDecl",
210-
},
211204
{
212205
R"cpp(
213206
struct S { S(const char*); };

0 commit comments

Comments
 (0)