diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -41,6 +41,7 @@ ClangdServer.cpp CodeComplete.cpp CodeCompletionStrings.cpp + CollectMacros.cpp CompileCommands.cpp Compiler.cpp Context.cpp diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h --- a/clang-tools-extra/clangd/CollectMacros.h +++ b/clang-tools-extra/clangd/CollectMacros.h @@ -40,10 +40,8 @@ /// - 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 @@ } 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/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp new file mode 100644 --- /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/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- 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::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 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 @@ 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 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 @@ 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 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -350,7 +350,7 @@ Macros = Preamble->Macros; Clang->getPreprocessor().addPPCallbacks( std::make_unique(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 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -54,7 +54,7 @@ return std::make_unique( collectIncludeStructureCallback(*SourceMgr, &Includes), - std::make_unique(*SourceMgr, *LangOpts, Macros)); + std::make_unique(*SourceMgr, Macros)); } CommentHandler *getCommentHandler() override { diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/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 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 sourceLocationInMainFile(const SourceManager &SM, diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -225,17 +225,6 @@ return true; } -llvm::Optional 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; } diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- 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 @@ return Result; } -llvm::Optional 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 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 @@ 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 &TargetDecls) : AST(AST) { for (const NamedDecl *D : TargetDecls) @@ -386,13 +393,17 @@ std::vector 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 @@ 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 CanonicalTargets; std::vector References; - const ASTContext &AST; + const ParsedAST &AST; }; std::vector findRefs(const std::vector &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 @@ // different kinds, deduplicate them. std::vector 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 @@ 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) {