kadircet updated this revision to Diff 246682.
kadircet added a comment.
Add more cleaning.
getDeclAtPosition builds a SelectionTree underneath and it only operates on
spelling locations inside main file. So it is invalid to pass any expansion
locations to it.
There is no need to call getBeginningOfIdentifier for offsets being thrown at
SelectionTrees.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75166/new/
https://reviews.llvm.org/D75166
Files:
clang-tools-extra/clangd/XRefs.cpp
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -214,24 +214,35 @@
}
}
+ auto CurLoc = sourceLocationInMainFile(SM, Pos);
+ if (!CurLoc) {
+ elog("locateSymbolAt failed to convert position to source location: {0}",
+ CurLoc.takeError());
+ return {};
+ }
+
+ const auto &TB = AST.getTokens();
// Macros are simple: there's no declaration/definition distinction.
// As a consequence, there's no need to look them up in the index either.
- SourceLocation IdentStartLoc = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(Pos, AST.getSourceManager(), AST.getLangOpts()));
std::vector<LocatedSymbol> Result;
- if (auto M = locateMacroAt(IdentStartLoc, AST.getPreprocessor())) {
- if (auto Loc = makeLocation(AST.getASTContext(),
- M->Info->getDefinitionLoc(), *MainFilePath)) {
- LocatedSymbol Macro;
- Macro.Name = std::string(M->Name);
- Macro.PreferredDeclaration = *Loc;
- Macro.Definition = Loc;
- Result.push_back(std::move(Macro));
-
- // Don't look at the AST or index if we have a macro result.
- // (We'd just return declarations referenced from the macro's
- // expansion.)
- return Result;
+ const auto *IdentifierCoveringCursor =
+ syntax::spelledIdentifierTouching(*CurLoc, TB);
+ if (IdentifierCoveringCursor) {
+ if (auto M = locateMacroAt(IdentifierCoveringCursor->location(),
+ AST.getPreprocessor())) {
+ if (auto Loc = makeLocation(AST.getASTContext(),
+ M->Info->getDefinitionLoc(), *MainFilePath)) {
+ LocatedSymbol Macro;
+ Macro.Name = std::string(M->Name);
+ Macro.PreferredDeclaration = *Loc;
+ Macro.Definition = Loc;
+ Result.push_back(std::move(Macro));
+
+ // Don't look at the AST or index if we have a macro result.
+ // (We'd just return declarations referenced from the macro's
+ // expansion.)
+ return Result;
+ }
}
}
@@ -244,15 +255,6 @@
// Keep track of SymbolID -> index mapping, to fill in index data later.
llvm::DenseMap<SymbolID, size_t> ResultIndex;
- SourceLocation SourceLoc;
- if (auto L = sourceLocationInMainFile(SM, Pos)) {
- SourceLoc = *L;
- } else {
- elog("locateSymbolAt failed to convert position to source location: {0}",
- L.takeError());
- return Result;
- }
-
auto AddResultDecl = [&](const NamedDecl *D) {
const NamedDecl *Def = getDefinition(D);
const NamedDecl *Preferred = Def ? Def : D;
@@ -277,16 +279,15 @@
// Emit all symbol locations (declaration or definition) from AST.
DeclRelationSet Relations =
DeclRelation::TemplatePattern | DeclRelation::Alias;
- for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
+ for (const NamedDecl *D : getDeclAtPosition(AST, *CurLoc, Relations)) {
// Special case: void foo() ^override: jump to the overridden method.
if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
- const InheritableAttr* Attr = D->getAttr<OverrideAttr>();
+ const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
if (!Attr)
Attr = D->getAttr<FinalAttr>();
- const syntax::Token *Tok =
- spelledIdentifierTouching(SourceLoc, AST.getTokens());
- if (Attr && Tok &&
- SM.getSpellingLoc(Attr->getLocation()) == Tok->location()) {
+ if (Attr && IdentifierCoveringCursor &&
+ SM.getSpellingLoc(Attr->getLocation()) ==
+ IdentifierCoveringCursor->location()) {
// We may be overridding multiple methods - offer them all.
for (const NamedDecl *ND : CMD->overridden_methods())
AddResultDecl(ND);
@@ -296,8 +297,9 @@
// Special case: the point of declaration of a template specialization,
// it's more useful to navigate to the template declaration.
- if (SM.getMacroArgExpandedLocation(D->getLocation()) == IdentStartLoc) {
- if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
+ if (IdentifierCoveringCursor &&
+ D->getLocation() == IdentifierCoveringCursor->location()) {
AddResultDecl(CTSD->getSpecializedTemplate());
continue;
}
@@ -416,12 +418,15 @@
// FIXME: show references to macro within file?
DeclRelationSet Relations =
DeclRelation::TemplatePattern | DeclRelation::Alias;
+ auto CurLoc = sourceLocationInMainFile(SM, Pos);
+ if (!CurLoc)
+ return {};
+ const auto *IdentifierAtCursor =
+ syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
+ if (!IdentifierAtCursor)
+ return {};
auto References = findRefs(
- getDeclAtPosition(AST,
- SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
- Pos, SM, AST.getLangOpts())),
- Relations),
- AST);
+ getDeclAtPosition(AST, IdentifierAtCursor->location(), Relations), AST);
// FIXME: we may get multiple DocumentHighlights with the same location and
// different kinds, deduplicate them.
@@ -456,11 +461,17 @@
return Results;
}
auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
- auto Loc = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
+ auto CurLoc = sourceLocationInMainFile(SM, Pos);
+ if (!CurLoc)
+ return {};
+ const auto *IdentifierAtCursor =
+ syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
+ if (!IdentifierAtCursor)
+ return {};
RefsRequest Req;
- if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+ if (auto Macro = locateMacroAt(IdentifierAtCursor->location(),
+ AST.getPreprocessor())) {
// Handle references to macro.
if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
// Collect macro references from main file.
@@ -483,7 +494,8 @@
// DeclRelation::Underlying.
DeclRelationSet Relations = DeclRelation::TemplatePattern |
DeclRelation::Alias | DeclRelation::Underlying;
- auto Decls = getDeclAtPosition(AST, Loc, Relations);
+ auto Decls =
+ getDeclAtPosition(AST, IdentifierAtCursor->location(), Relations);
// We traverse the AST to find references in the main file.
auto MainFileRefs = findRefs(Decls, AST);
@@ -747,17 +759,16 @@
};
const SourceManager &SM = AST.getSourceManager();
- SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- unsigned Offset = SM.getDecomposedSpellingLoc(SourceLocationBeg).second;
const CXXRecordDecl *Result = nullptr;
- SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
- Offset, [&](SelectionTree ST) {
+ auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
+ if (!Offset)
+ return Result;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), *Offset,
+ *Offset, [&](SelectionTree ST) {
Result = RecordFromNode(ST.commonAncestor());
return Result != nullptr;
});
return Result;
-
}
std::vector<const CXXRecordDecl *> typeParents(const CXXRecordDecl *CXXRD) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
