[clang-tools-extra] 3755039 - [clangd] Get rid of getTokenRange helper
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 3 05:30:55 PST 2020
Author: Kadir Cetinkaya
Date: 2020-03-03T14:30:42+01:00
New Revision: 3755039c99d85f93168cb7a36501c9586c19d3db
URL: https://github.com/llvm/llvm-project/commit/3755039c99d85f93168cb7a36501c9586c19d3db
DIFF: https://github.com/llvm/llvm-project/commit/3755039c99d85f93168cb7a36501c9586c19d3db.diff
LOG: [clangd] Get rid of getTokenRange helper
Summary:
Reviewers: sammccall
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75474
Added:
clang-tools-extra/clangd/CollectMacros.cpp
Modified:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/XRefs.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
index fc5a07e69e9d..5db345ecc63f 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -41,6 +41,7 @@ add_clang_library(clangDaemon
ClangdServer.cpp
CodeComplete.cpp
CodeCompletionStrings.cpp
+ CollectMacros.cpp
CompileCommands.cpp
Compiler.cpp
Context.cpp
diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp
new file mode 100644
index 000000000000..ea7dd18ee130
--- /dev/null
+++ b/clang-tools-extra/clangd/CollectMacros.cpp
@@ -0,0 +1,34 @@
+//===--- CollectMacros.cpp ---------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CollectMacros.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+
+void CollectMainFileMacros::add(const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ if (!InMainFile)
+ return;
+ auto Loc = MacroNameTok.getLocation();
+ if (Loc.isInvalid() || Loc.isMacroID())
+ return;
+
+ auto Name = MacroNameTok.getIdentifierInfo()->getName();
+ Out.Names.insert(Name);
+ auto Range = halfOpenToRange(
+ SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
+ if (auto SID = getSymbolID(Name, MI, SM))
+ Out.MacroRefs[*SID].push_back(Range);
+ else
+ Out.UnknownMacros.push_back(Range);
+}
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h
index 5c3fca10ad4a..eecea0455be2 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -40,10 +40,8 @@ struct MainFileMacros {
/// - collect macros after the preamble of the main file (in ParsedAST.cpp)
class CollectMainFileMacros : public PPCallbacks {
public:
- explicit CollectMainFileMacros(const SourceManager &SM,
- const LangOptions &LangOpts,
- MainFileMacros &Out)
- : SM(SM), LangOpts(LangOpts), Out(Out) {}
+ explicit CollectMainFileMacros(const SourceManager &SM, MainFileMacros &Out)
+ : SM(SM), Out(Out) {}
void FileChanged(SourceLocation Loc, FileChangeReason,
SrcMgr::CharacteristicKind, FileID) override {
@@ -89,24 +87,8 @@ class CollectMainFileMacros : public PPCallbacks {
}
private:
- void add(const Token &MacroNameTok, const MacroInfo *MI) {
- if (!InMainFile)
- return;
- auto Loc = MacroNameTok.getLocation();
- if (Loc.isMacroID())
- return;
-
- if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
- auto Name = MacroNameTok.getIdentifierInfo()->getName();
- Out.Names.insert(Name);
- if (auto SID = getSymbolID(Name, MI, SM))
- Out.MacroRefs[*SID].push_back(*Range);
- else
- Out.UnknownMacros.push_back(*Range);
- }
- }
+ void add(const Token &MacroNameTok, const MacroInfo *MI);
const SourceManager &SM;
- const LangOptions &LangOpts;
bool InMainFile = true;
MainFileMacros &Out;
};
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 5796657a5800..5c1288c14b58 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -26,6 +26,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Index/IndexSymbol.h"
@@ -530,32 +531,33 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
llvm::consumeError(CurLoc.takeError());
return llvm::None;
}
- auto TokensTouchingCursor =
- syntax::spelledTokensTouching(*CurLoc, AST.getTokens());
+ const auto &TB = AST.getTokens();
+ auto TokensTouchingCursor = syntax::spelledTokensTouching(*CurLoc, TB);
// Early exit if there were no tokens around the cursor.
if (TokensTouchingCursor.empty())
return llvm::None;
- // To be used as a backup for highlighting the selected token.
- SourceLocation IdentLoc;
+ // To be used as a backup for highlighting the selected token, we use back as
+ // it aligns better with biases elsewhere (editors tend to send the position
+ // for the left of the hovered token).
+ CharSourceRange HighlightRange =
+ TokensTouchingCursor.back().range(SM).toCharRange(SM);
llvm::Optional<HoverInfo> HI;
// Macros and deducedtype only works on identifiers and auto/decltype keywords
// respectively. Therefore they are only trggered on whichever works for them,
// similar to SelectionTree::create().
for (const auto &Tok : TokensTouchingCursor) {
if (Tok.kind() == tok::identifier) {
- IdentLoc = Tok.location();
+ // Prefer the identifier token as a fallback highlighting range.
+ HighlightRange = Tok.range(SM).toCharRange(SM);
if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
HI = getHoverContents(*M, AST);
- HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
- Tok.location());
break;
}
} else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
if (auto Deduced = getDeducedType(AST.getASTContext(), Tok.location())) {
HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
- HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
- Tok.location());
+ HighlightRange = Tok.range(SM).toCharRange(SM);
break;
}
}
@@ -566,10 +568,11 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
auto Offset = SM.getFileOffset(*CurLoc);
// Editors send the position on the left of the hovered character.
// So our selection tree should be biased right. (Tested with VSCode).
- SelectionTree ST = SelectionTree::createRight(
- AST.getASTContext(), AST.getTokens(), Offset, Offset);
+ SelectionTree ST =
+ SelectionTree::createRight(AST.getASTContext(), TB, Offset, Offset);
std::vector<const Decl *> Result;
if (const SelectionTree::Node *N = ST.commonAncestor()) {
+ // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
if (!Decls.empty()) {
HI = getHoverContents(Decls.front(), Index);
@@ -592,14 +595,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
if (auto Formatted =
tooling::applyAllReplacements(HI->Definition, Replacements))
HI->Definition = *Formatted;
- // FIXME: We should rather fill this with info coming from SelectionTree node.
- if (!HI->SymRange) {
- SourceLocation ToHighlight = TokensTouchingCursor.front().location();
- if (IdentLoc.isValid())
- ToHighlight = IdentLoc;
- HI->SymRange =
- getTokenRange(AST.getSourceManager(), AST.getLangOpts(), ToHighlight);
- }
+ HI->SymRange = halfOpenToRange(SM, HighlightRange);
return HI;
}
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 36a9c47f7a9d..e43c2ce66261 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -350,7 +350,7 @@ ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
Macros = Preamble->Macros;
Clang->getPreprocessor().addPPCallbacks(
std::make_unique<CollectMainFileMacros>(Clang->getSourceManager(),
- Clang->getLangOpts(), Macros));
+ Macros));
// Copy over the includes from the preamble, then combine with the
// non-preamble includes below.
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index eca545fd09e4..f2b6b017f10f 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -54,7 +54,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
return std::make_unique<PPChainedCallbacks>(
collectIncludeStructureCallback(*SourceMgr, &Includes),
- std::make_unique<CollectMainFileMacros>(*SourceMgr, *LangOpts, Macros));
+ std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros));
}
CommentHandler *getCommentHandler() override {
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 79d027def4bc..d18daa910d18 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -225,17 +225,6 @@ bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
return true;
}
-llvm::Optional<Range> getTokenRange(const SourceManager &SM,
- const LangOptions &LangOpts,
- SourceLocation TokLoc) {
- if (!TokLoc.isValid())
- return llvm::None;
- SourceLocation End = Lexer::getLocForEndOfToken(TokLoc, 0, SM, LangOpts);
- if (!End.isValid())
- return llvm::None;
- return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End));
-}
-
bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {
if (!R.getBegin().isValid() || !R.getEnd().isValid())
return false;
@@ -645,8 +634,7 @@ std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
[&](const syntax::Token &Tok, const SourceManager &SM) {
if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
return;
- if (auto Range = getTokenRange(SM, LangOpts, Tok.location()))
- Ranges.push_back(*Range);
+ Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
});
return Ranges;
}
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index c601cc89df28..383c57371b00 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -69,11 +69,6 @@ Position offsetToPosition(llvm::StringRef Code, size_t Offset);
/// FIXME: This should return an error if the location is invalid.
Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
-/// Returns the taken range at \p TokLoc.
-llvm::Optional<Range> getTokenRange(const SourceManager &SM,
- const LangOptions &LangOpts,
- SourceLocation TokLoc);
-
/// Return the file location, corresponding to \p P. Note that one should take
/// care to avoid comparing the result with expansion locations.
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 560e39e13550..67f7bda6a5e6 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -29,6 +29,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Type.h"
#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Index/IndexDataConsumer.h"
@@ -149,25 +150,27 @@ std::vector<const NamedDecl *> getDeclAtPosition(ParsedAST &AST,
return Result;
}
-llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
+// Expects Loc to be a SpellingLocation, will bail out otherwise as it can't
+// figure out a filename.
+llvm::Optional<Location> makeLocation(const ASTContext &AST, SourceLocation Loc,
llvm::StringRef TUPath) {
- const SourceManager &SourceMgr = AST.getSourceManager();
- const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
+ const auto &SM = AST.getSourceManager();
+ const FileEntry *F = SM.getFileEntryForID(SM.getFileID(Loc));
if (!F)
return None;
- auto FilePath = getCanonicalPath(F, SourceMgr);
+ auto FilePath = getCanonicalPath(F, SM);
if (!FilePath) {
log("failed to get path!");
return None;
}
- if (auto Range =
- getTokenRange(AST.getSourceManager(), AST.getLangOpts(), TokLoc)) {
- Location L;
- L.uri = URIForFile::canonicalize(*FilePath, TUPath);
- L.range = *Range;
- return L;
- }
- return None;
+ Location L;
+ L.uri = URIForFile::canonicalize(*FilePath, TUPath);
+ // We call MeasureTokenLength here as TokenBuffer doesn't store spelled tokens
+ // outside the main file.
+ auto TokLen = Lexer::MeasureTokenLength(Loc, SM, AST.getLangOpts());
+ L.range = halfOpenToRange(
+ SM, CharSourceRange::getCharRange(Loc, Loc.getLocWithOffset(TokLen)));
+ return L;
}
} // namespace
@@ -373,11 +376,15 @@ namespace {
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
- SourceLocation Loc;
+ syntax::Token SpelledTok;
index::SymbolRoleSet Role;
+
+ Range range(const SourceManager &SM) const {
+ return halfOpenToRange(SM, SpelledTok.range(SM).toCharRange(SM));
+ }
};
- ReferenceFinder(ASTContext &AST, Preprocessor &PP,
+ ReferenceFinder(const ParsedAST &AST,
const std::vector<const NamedDecl *> &TargetDecls)
: AST(AST) {
for (const NamedDecl *D : TargetDecls)
@@ -386,13 +393,17 @@ class ReferenceFinder : public index::IndexDataConsumer {
std::vector<Reference> take() && {
llvm::sort(References, [](const Reference &L, const Reference &R) {
- return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
+ auto LTok = L.SpelledTok.location();
+ auto RTok = R.SpelledTok.location();
+ return std::tie(LTok, L.Role) < std::tie(RTok, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
References.erase(std::unique(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
- return std::tie(L.Loc, L.Role) ==
- std::tie(R.Loc, R.Role);
+ auto LTok = L.SpelledTok.location();
+ auto RTok = R.SpelledTok.location();
+ return std::tie(LTok, L.Role) ==
+ std::tie(RTok, R.Role);
}),
References.end());
return std::move(References);
@@ -404,22 +415,27 @@ class ReferenceFinder : public index::IndexDataConsumer {
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
+ if (!CanonicalTargets.count(D))
+ return true;
+ const auto &TB = AST.getTokens();
const SourceManager &SM = AST.getSourceManager();
Loc = SM.getFileLoc(Loc);
- if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
- References.push_back({Loc, Roles});
+ // We are only traversing decls *inside* the main file, so this should hold.
+ assert(isInsideMainFile(Loc, SM));
+ if (const auto *Tok = TB.spelledTokenAt(Loc))
+ References.push_back({*Tok, Roles});
return true;
}
private:
llvm::SmallSet<const Decl *, 4> CanonicalTargets;
std::vector<Reference> References;
- const ASTContext &AST;
+ const ParsedAST &AST;
};
std::vector<ReferenceFinder::Reference>
findRefs(const std::vector<const NamedDecl *> &Decls, ParsedAST &AST) {
- ReferenceFinder RefFinder(AST.getASTContext(), AST.getPreprocessor(), Decls);
+ ReferenceFinder RefFinder(AST, Decls);
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -450,18 +466,15 @@ std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
//
diff erent kinds, deduplicate them.
std::vector<DocumentHighlight> Result;
for (const auto &Ref : References) {
- if (auto Range =
- getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
- DocumentHighlight DH;
- DH.range = *Range;
- if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
- DH.kind = DocumentHighlightKind::Write;
- else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
- DH.kind = DocumentHighlightKind::Read;
- else
- DH.kind = DocumentHighlightKind::Text;
- Result.push_back(std::move(DH));
- }
+ DocumentHighlight DH;
+ DH.range = Ref.range(SM);
+ if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
+ DH.kind = DocumentHighlightKind::Write;
+ else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
+ DH.kind = DocumentHighlightKind::Read;
+ else
+ DH.kind = DocumentHighlightKind::Text;
+ Result.push_back(std::move(DH));
}
return Result;
}
@@ -524,16 +537,15 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
[](const ReferenceFinder::Reference &L,
const ReferenceFinder::Reference &R) {
- return L.Loc == R.Loc;
+ return L.SpelledTok.location() ==
+ R.SpelledTok.location();
}),
MainFileRefs.end());
for (const auto &Ref : MainFileRefs) {
- if (auto Range = getTokenRange(SM, AST.getLangOpts(), Ref.Loc)) {
- Location Result;
- Result.range = *Range;
- Result.uri = URIMainFile;
- Results.References.push_back(std::move(Result));
- }
+ Location Result;
+ Result.range = Ref.range(SM);
+ Result.uri = URIMainFile;
+ Results.References.push_back(std::move(Result));
}
if (Index && Results.References.size() <= Limit) {
for (const Decl *D : Decls) {
More information about the cfe-commits
mailing list