Skip to content

Commit 2011d14

Browse files
committed
[clangd] Clean-up XRefs.cpp from Lexer usages and unnecessary SourceLoc transformations
Summary: Get rid of calls to lexer and unnecessary source location transformations. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D75166
1 parent b3d0c79 commit 2011d14

File tree

2 files changed

+89
-52
lines changed

2 files changed

+89
-52
lines changed

clang-tools-extra/clangd/XRefs.cpp

Lines changed: 71 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "llvm/ADT/StringExtras.h"
4444
#include "llvm/ADT/StringRef.h"
4545
#include "llvm/Support/Casting.h"
46+
#include "llvm/Support/Error.h"
4647
#include "llvm/Support/Path.h"
4748
#include "llvm/Support/raw_ostream.h"
4849

@@ -214,24 +215,34 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
214215
}
215216
}
216217

218+
auto CurLoc = sourceLocationInMainFile(SM, Pos);
219+
if (!CurLoc) {
220+
elog("locateSymbolAt failed to convert position to source location: {0}",
221+
CurLoc.takeError());
222+
return {};
223+
}
224+
217225
// Macros are simple: there's no declaration/definition distinction.
218226
// As a consequence, there's no need to look them up in the index either.
219-
SourceLocation IdentStartLoc = SM.getMacroArgExpandedLocation(
220-
getBeginningOfIdentifier(Pos, AST.getSourceManager(), AST.getLangOpts()));
221227
std::vector<LocatedSymbol> Result;
222-
if (auto M = locateMacroAt(IdentStartLoc, AST.getPreprocessor())) {
223-
if (auto Loc = makeLocation(AST.getASTContext(),
224-
M->Info->getDefinitionLoc(), *MainFilePath)) {
225-
LocatedSymbol Macro;
226-
Macro.Name = std::string(M->Name);
227-
Macro.PreferredDeclaration = *Loc;
228-
Macro.Definition = Loc;
229-
Result.push_back(std::move(Macro));
230-
231-
// Don't look at the AST or index if we have a macro result.
232-
// (We'd just return declarations referenced from the macro's
233-
// expansion.)
234-
return Result;
228+
const auto *TouchedIdentifier =
229+
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
230+
if (TouchedIdentifier) {
231+
if (auto M = locateMacroAt(TouchedIdentifier->location(),
232+
AST.getPreprocessor())) {
233+
if (auto Loc = makeLocation(AST.getASTContext(),
234+
M->Info->getDefinitionLoc(), *MainFilePath)) {
235+
LocatedSymbol Macro;
236+
Macro.Name = std::string(M->Name);
237+
Macro.PreferredDeclaration = *Loc;
238+
Macro.Definition = Loc;
239+
Result.push_back(std::move(Macro));
240+
241+
// Don't look at the AST or index if we have a macro result.
242+
// (We'd just return declarations referenced from the macro's
243+
// expansion.)
244+
return Result;
245+
}
235246
}
236247
}
237248

@@ -244,15 +255,6 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
244255
// Keep track of SymbolID -> index mapping, to fill in index data later.
245256
llvm::DenseMap<SymbolID, size_t> ResultIndex;
246257

247-
SourceLocation SourceLoc;
248-
if (auto L = sourceLocationInMainFile(SM, Pos)) {
249-
SourceLoc = *L;
250-
} else {
251-
elog("locateSymbolAt failed to convert position to source location: {0}",
252-
L.takeError());
253-
return Result;
254-
}
255-
256258
auto AddResultDecl = [&](const NamedDecl *D) {
257259
const NamedDecl *Def = getDefinition(D);
258260
const NamedDecl *Preferred = Def ? Def : D;
@@ -277,16 +279,15 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
277279
// Emit all symbol locations (declaration or definition) from AST.
278280
DeclRelationSet Relations =
279281
DeclRelation::TemplatePattern | DeclRelation::Alias;
280-
for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
282+
for (const NamedDecl *D : getDeclAtPosition(AST, *CurLoc, Relations)) {
281283
// Special case: void foo() ^override: jump to the overridden method.
282284
if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
283-
const InheritableAttr* Attr = D->getAttr<OverrideAttr>();
285+
const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
284286
if (!Attr)
285287
Attr = D->getAttr<FinalAttr>();
286-
const syntax::Token *Tok =
287-
spelledIdentifierTouching(SourceLoc, AST.getTokens());
288-
if (Attr && Tok &&
289-
SM.getSpellingLoc(Attr->getLocation()) == Tok->location()) {
288+
if (Attr && TouchedIdentifier &&
289+
SM.getSpellingLoc(Attr->getLocation()) ==
290+
TouchedIdentifier->location()) {
290291
// We may be overridding multiple methods - offer them all.
291292
for (const NamedDecl *ND : CMD->overridden_methods())
292293
AddResultDecl(ND);
@@ -296,8 +297,9 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
296297

297298
// Special case: the point of declaration of a template specialization,
298299
// it's more useful to navigate to the template declaration.
299-
if (SM.getMacroArgExpandedLocation(D->getLocation()) == IdentStartLoc) {
300-
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
300+
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
301+
if (TouchedIdentifier &&
302+
D->getLocation() == TouchedIdentifier->location()) {
301303
AddResultDecl(CTSD->getSpecializedTemplate());
302304
continue;
303305
}
@@ -416,12 +418,12 @@ std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
416418
// FIXME: show references to macro within file?
417419
DeclRelationSet Relations =
418420
DeclRelation::TemplatePattern | DeclRelation::Alias;
419-
auto References = findRefs(
420-
getDeclAtPosition(AST,
421-
SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
422-
Pos, SM, AST.getLangOpts())),
423-
Relations),
424-
AST);
421+
auto CurLoc = sourceLocationInMainFile(SM, Pos);
422+
if (!CurLoc) {
423+
llvm::consumeError(CurLoc.takeError());
424+
return {};
425+
}
426+
auto References = findRefs(getDeclAtPosition(AST, *CurLoc, Relations), AST);
425427

426428
// FIXME: we may get multiple DocumentHighlights with the same location and
427429
// different kinds, deduplicate them.
@@ -456,11 +458,18 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
456458
return Results;
457459
}
458460
auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
459-
auto Loc = SM.getMacroArgExpandedLocation(
460-
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
461+
auto CurLoc = sourceLocationInMainFile(SM, Pos);
462+
if (!CurLoc) {
463+
llvm::consumeError(CurLoc.takeError());
464+
return {};
465+
}
466+
SourceLocation SLocId;
467+
if (const auto *IdentifierAtCursor =
468+
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens()))
469+
SLocId = IdentifierAtCursor->location();
461470
RefsRequest Req;
462471

463-
if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
472+
if (auto Macro = locateMacroAt(SLocId, AST.getPreprocessor())) {
464473
// Handle references to macro.
465474
if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
466475
// Collect macro references from main file.
@@ -483,7 +492,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
483492
// DeclRelation::Underlying.
484493
DeclRelationSet Relations = DeclRelation::TemplatePattern |
485494
DeclRelation::Alias | DeclRelation::Underlying;
486-
auto Decls = getDeclAtPosition(AST, Loc, Relations);
495+
auto Decls = getDeclAtPosition(AST, *CurLoc, Relations);
487496

488497
// We traverse the AST to find references in the main file.
489498
auto MainFileRefs = findRefs(Decls, AST);
@@ -541,16 +550,19 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
541550

542551
std::vector<SymbolDetails> getSymbolInfo(ParsedAST &AST, Position Pos) {
543552
const SourceManager &SM = AST.getSourceManager();
544-
auto Loc = SM.getMacroArgExpandedLocation(
545-
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
553+
auto CurLoc = sourceLocationInMainFile(SM, Pos);
554+
if (!CurLoc) {
555+
llvm::consumeError(CurLoc.takeError());
556+
return {};
557+
}
546558

547559
std::vector<SymbolDetails> Results;
548560

549561
// We also want the targets of using-decls, so we include
550562
// DeclRelation::Underlying.
551563
DeclRelationSet Relations = DeclRelation::TemplatePattern |
552564
DeclRelation::Alias | DeclRelation::Underlying;
553-
for (const NamedDecl *D : getDeclAtPosition(AST, Loc, Relations)) {
565+
for (const NamedDecl *D : getDeclAtPosition(AST, *CurLoc, Relations)) {
554566
SymbolDetails NewSymbol;
555567
std::string QName = printQualifiedName(*D);
556568
auto SplitQName = splitQualifiedName(QName);
@@ -570,7 +582,13 @@ std::vector<SymbolDetails> getSymbolInfo(ParsedAST &AST, Position Pos) {
570582
Results.push_back(std::move(NewSymbol));
571583
}
572584

573-
if (auto M = locateMacroAt(Loc, AST.getPreprocessor())) {
585+
const auto *IdentifierAtCursor =
586+
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
587+
if (!IdentifierAtCursor)
588+
return Results;
589+
590+
if (auto M = locateMacroAt(IdentifierAtCursor->location(),
591+
AST.getPreprocessor())) {
574592
SymbolDetails NewMacro;
575593
NewMacro.name = std::string(M->Name);
576594
llvm::SmallString<32> USR;
@@ -747,17 +765,18 @@ const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) {
747765
};
748766

749767
const SourceManager &SM = AST.getSourceManager();
750-
SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
751-
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
752-
unsigned Offset = SM.getDecomposedSpellingLoc(SourceLocationBeg).second;
753768
const CXXRecordDecl *Result = nullptr;
754-
SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
755-
Offset, [&](SelectionTree ST) {
769+
auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
770+
if (!Offset) {
771+
llvm::consumeError(Offset.takeError());
772+
return Result;
773+
}
774+
SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), *Offset,
775+
*Offset, [&](SelectionTree ST) {
756776
Result = RecordFromNode(ST.commonAncestor());
757777
return Result != nullptr;
758778
});
759779
return Result;
760-
761780
}
762781

763782
std::vector<const CXXRecordDecl *> typeParents(const CXXRecordDecl *CXXRD) {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,15 @@ TEST(HighlightsTest, All) {
9797
R"cpp(// Function parameter in decl
9898
void foo(int [[^bar]]);
9999
)cpp",
100+
R"cpp(// Not touching any identifiers.
101+
struct Foo {
102+
[[~]]Foo() {};
103+
};
104+
void foo() {
105+
Foo f;
106+
f.[[^~]]Foo();
107+
}
108+
)cpp",
100109
};
101110
for (const char *Test : Tests) {
102111
Annotations T(Test);
@@ -960,6 +969,15 @@ TEST(FindReferences, WithinAST) {
960969
class [[Foo]] {};
961970
void func([[Fo^o]]<int>);
962971
)cpp",
972+
R"cpp(// Not touching any identifiers.
973+
struct Foo {
974+
[[~]]Foo() {};
975+
};
976+
void foo() {
977+
Foo f;
978+
f.[[^~]]Foo();
979+
}
980+
)cpp",
963981
};
964982
for (const char *Test : Tests) {
965983
Annotations T(Test);

0 commit comments

Comments
 (0)