kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov, mgorny.
Herald added a project: clang.
kadircet added a parent revision: D75447: [clangd] Make use of token buffers in
semantic highlighting.
kadircet added a parent revision: D75446: [clang][Syntax] Handle macro
arguments in spelledForExpanded.
There's only one caller left in CollectMacros, which doesn't have
access to TokenBuffers. I believe we should just stop computing ranges in there,
and collect offsets instead. Later on we can convert them to Ranges when we've
got access to TokenBuffers. As those ranges are never used without a ParsedAST.
But leaving it out for now to keep the patch minimal.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75474
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
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
@@ -152,7 +152,12 @@
llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
llvm::StringRef TUPath) {
const SourceManager &SourceMgr = AST.getSourceManager();
- const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
+ auto Tok = syntax::tokenize(syntax::FileRange(SourceMgr, TokLoc, 1),
+ SourceMgr, AST.getLangOpts());
+ if (Tok.empty())
+ return None;
+ auto Range = Tok.front().range(SourceMgr);
+ const FileEntry *F = SourceMgr.getFileEntryForID(Range.file());
if (!F)
return None;
auto FilePath = getCanonicalPath(F, SourceMgr);
@@ -160,14 +165,10 @@
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);
+ L.range = halfOpenToRange(SourceMgr, Range.toCharRange(SourceMgr));
+ return L;
}
} // namespace
@@ -351,11 +352,11 @@
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
- SourceLocation Loc;
+ syntax::Token Tok;
index::SymbolRoleSet Role;
};
- ReferenceFinder(ASTContext &AST, Preprocessor &PP,
+ ReferenceFinder(const ParsedAST &AST,
const std::vector<const NamedDecl *> &TargetDecls)
: AST(AST) {
for (const NamedDecl *D : TargetDecls)
@@ -364,13 +365,17 @@
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.Tok.location();
+ auto RTok = R.Tok.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.Tok.location();
+ auto RTok = R.Tok.location();
+ return std::tie(LTok, L.Role) ==
+ std::tie(RTok, R.Role);
}),
References.end());
return std::move(References);
@@ -382,22 +387,27 @@
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});
+ auto Toks = TB.spelledForExpanded(TB.expandedTokens(Loc));
+ if (!Toks || Toks->empty() ||
+ !isInsideMainFile(Toks->front().location(), SM))
+ return true;
+ References.push_back({Toks->front(), 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;
@@ -428,18 +438,15 @@
// different 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 = halfOpenToRange(SM, Ref.Tok.range(SM).toCharRange(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;
}
@@ -502,16 +509,15 @@
MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
[](const ReferenceFinder::Reference &L,
const ReferenceFinder::Reference &R) {
- return L.Loc == R.Loc;
+ return L.Tok.location() ==
+ R.Tok.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 = halfOpenToRange(SM, Ref.Tok.range(SM).toCharRange(SM));
+ Result.uri = URIMainFile;
+ Results.References.push_back(std::move(Result));
}
if (Index && Results.References.size() <= Limit) {
for (const Decl *D : Decls) {
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -69,11 +69,6 @@
/// 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,
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -225,17 +225,6 @@
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 @@
[&](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;
}
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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,31 @@
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;
+ CharSourceRange HighlightRange =
+ TokensTouchingCursor.front().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 +566,11 @@
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 +593,7 @@
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;
}
Index: clang-tools-extra/clangd/CollectMacros.h
===================================================================
--- clang-tools-extra/clangd/CollectMacros.h
+++ clang-tools-extra/clangd/CollectMacros.h
@@ -89,22 +89,7 @@
}
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;
Index: clang-tools-extra/clangd/CollectMacros.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/CollectMacros.cpp
@@ -0,0 +1,46 @@
+//===--- 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/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Returns the taken range at \p TokLoc.
+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));
+}
+} // namespace
+
+void CollectMainFileMacros::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);
+ }
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -41,6 +41,7 @@
ClangdServer.cpp
CodeComplete.cpp
CodeCompletionStrings.cpp
+ CollectMacros.cpp
CompileCommands.cpp
Compiler.cpp
Context.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
