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 @@ -27,7 +27,9 @@ #include "clang/AST/PrettyPrinter.h" #include "clang/AST/Type.h" #include "clang/Basic/Specifiers.h" +#include "clang/Basic/TokenKinds.h" #include "clang/Index/IndexSymbol.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" @@ -523,24 +525,44 @@ format::FormatStyle Style, const SymbolIndex *Index) { const SourceManager &SM = AST.getSourceManager(); - llvm::Optional HI; - SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation( - getBeginningOfIdentifier(Pos, SM, AST.getLangOpts())); + auto CurLoc = sourceLocationInMainFile(SM, Pos); + if (!CurLoc) { + llvm::consumeError(CurLoc.takeError()); + return llvm::None; + } + auto TokensAroundCursor = + syntax::spelledTokensTouching(*CurLoc, AST.getTokens()); + if (TokensAroundCursor.empty()) + return llvm::None; - if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) { + // In general we prefer the touching token that works over the one that + // doesn't, see SelectionTree::create(). The following locations are used only + // for triggering on macros and auto/decltype, so simply choosing the lone + // identifier-or-keyword token is equivalent. + SourceLocation SLocID; + SourceLocation SLocAutoKW; + for (const auto &Tok : TokensAroundCursor) { + if (Tok.kind() == tok::identifier) + SLocID = Tok.location(); + if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) + SLocAutoKW = Tok.location(); + } + + llvm::Optional HI; + if (auto Deduced = getDeducedType(AST.getASTContext(), SLocAutoKW)) { HI = getHoverContents(*Deduced, AST.getASTContext(), Index); - } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) { + HI->SymRange = + getTokenRange(AST.getSourceManager(), AST.getLangOpts(), SLocAutoKW); + } else if (auto M = locateMacroAt(SLocID, AST.getPreprocessor())) { HI = getHoverContents(*M, AST); + HI->SymRange = + getTokenRange(AST.getSourceManager(), AST.getLangOpts(), SLocID); } else { - auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos); - if (!Offset) { - llvm::consumeError(Offset.takeError()); - return llvm::None; - } + 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); + AST.getASTContext(), AST.getTokens(), Offset, Offset); std::vector Result; if (const SelectionTree::Node *N = ST.commonAncestor()) { auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias); @@ -565,9 +587,15 @@ 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 = TokensAroundCursor.front().location(); + if (SLocID.isValid()) + ToHighlight = SLocID; + HI->SymRange = + getTokenRange(AST.getSourceManager(), AST.getLangOpts(), ToHighlight); + } - HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(), - SourceLocationBeg); return HI; } 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 @@ -78,14 +78,6 @@ llvm::Expected sourceLocationInMainFile(const SourceManager &SM, Position P); -/// Get the beginning SourceLocation at a specified \p Pos in the main file. -/// May be invalid if Pos is, or if there's no identifier or operators. -/// The returned position is in the main file, callers may prefer to -/// obtain the macro expansion location. -SourceLocation getBeginningOfIdentifier(const Position &Pos, - const SourceManager &SM, - const LangOptions &LangOpts); - /// Returns true iff \p Loc is inside the main file. This function handles /// file & macro locations. For macro locations, returns iff the macro is being /// expanded inside the main file. 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 @@ -235,108 +235,6 @@ return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End)); } -namespace { - -enum TokenFlavor { Identifier, Operator, Whitespace, Other }; - -bool isOverloadedOperator(const Token &Tok) { - switch (Tok.getKind()) { -#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemOnly) \ - case tok::Token: -#define OVERLOADED_OPERATOR_MULTI(Name, Spelling, Unary, Binary, MemOnly) -#include "clang/Basic/OperatorKinds.def" - return true; - - default: - break; - } - return false; -} - -TokenFlavor getTokenFlavor(SourceLocation Loc, const SourceManager &SM, - const LangOptions &LangOpts) { - Token Tok; - Tok.setKind(tok::NUM_TOKENS); - if (Lexer::getRawToken(Loc, Tok, SM, LangOpts, - /*IgnoreWhiteSpace*/ false)) - return Other; - - // getRawToken will return false without setting Tok when the token is - // whitespace, so if the flag is not set, we are sure this is a whitespace. - if (Tok.is(tok::TokenKind::NUM_TOKENS)) - return Whitespace; - if (Tok.is(tok::TokenKind::raw_identifier)) - return Identifier; - if (isOverloadedOperator(Tok)) - return Operator; - return Other; -} - -} // namespace - -SourceLocation getBeginningOfIdentifier(const Position &Pos, - const SourceManager &SM, - const LangOptions &LangOpts) { - FileID FID = SM.getMainFileID(); - auto Offset = positionToOffset(SM.getBufferData(FID), Pos); - if (!Offset) { - log("getBeginningOfIdentifier: {0}", Offset.takeError()); - return SourceLocation(); - } - - // GetBeginningOfToken(InputLoc) is almost what we want, but does the wrong - // thing if the cursor is at the end of the token (identifier or operator). - // The cases are: - // 1) at the beginning of the token - // 2) at the middle of the token - // 3) at the end of the token - // 4) anywhere outside the identifier or operator - // To distinguish all cases, we lex both at the - // GetBeginningOfToken(InputLoc-1) and GetBeginningOfToken(InputLoc), for - // cases 1 and 4, we just return the original location. - SourceLocation InputLoc = SM.getComposedLoc(FID, *Offset); - if (*Offset == 0) // Case 1 or 4. - return InputLoc; - SourceLocation Before = SM.getComposedLoc(FID, *Offset - 1); - SourceLocation BeforeTokBeginning = - Lexer::GetBeginningOfToken(Before, SM, LangOpts); - TokenFlavor BeforeKind = getTokenFlavor(BeforeTokBeginning, SM, LangOpts); - - SourceLocation CurrentTokBeginning = - Lexer::GetBeginningOfToken(InputLoc, SM, LangOpts); - TokenFlavor CurrentKind = getTokenFlavor(CurrentTokBeginning, SM, LangOpts); - - // At the middle of the token. - if (BeforeTokBeginning == CurrentTokBeginning) { - // For interesting token, we return the beginning of the token. - if (CurrentKind == Identifier || CurrentKind == Operator) - return CurrentTokBeginning; - // otherwise, we return the original loc. - return InputLoc; - } - - // Whitespace is not interesting. - if (BeforeKind == Whitespace) - return CurrentTokBeginning; - if (CurrentKind == Whitespace) - return BeforeTokBeginning; - - // The cursor is at the token boundary, e.g. "Before^Current", we prefer - // identifiers to other tokens. - if (CurrentKind == Identifier) - return CurrentTokBeginning; - if (BeforeKind == Identifier) - return BeforeTokBeginning; - // Then prefer overloaded operators to other tokens. - if (CurrentKind == Operator) - return CurrentTokBeginning; - if (BeforeKind == Operator) - return BeforeTokBeginning; - - // Non-interesting case, we just return the original location. - return InputLoc; -} - bool isValidFileRange(const SourceManager &Mgr, SourceRange R) { if (!R.getBegin().isValid() || !R.getEnd().isValid()) return false; diff --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp --- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp +++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp @@ -87,9 +87,9 @@ if (ExpectedRefs.empty()) break; - auto Loc = getBeginningOfIdentifier(ExpectedRefs.begin()->start, SM, - AST.getLangOpts()); - auto Macro = locateMacroAt(Loc, PP); + auto Loc = sourceLocationInMainFile(SM, ExpectedRefs.begin()->start); + ASSERT_TRUE(bool(Loc)); + auto Macro = locateMacroAt(*Loc, PP); assert(Macro); auto SID = getSymbolID(Macro->Name, Macro->Info, SM); diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -312,60 +312,6 @@ } } -TEST(SourceCodeTests, GetBeginningOfIdentifier) { - std::string Preamble = R"cpp( -struct Bar { int func(); }; -#define MACRO(X) void f() { X; } -Bar* bar; - )cpp"; - // First ^ is the expected beginning, last is the search position. - for (const std::string &Text : std::vector{ - "int ^f^oo();", // inside identifier - "int ^foo();", // beginning of identifier - "int ^foo^();", // end of identifier - "int foo(^);", // non-identifier - "^int foo();", // beginning of file (can't back up) - "int ^f0^0();", // after a digit (lexing at N-1 is wrong) - "/^/ comments", // non-interesting token - "void f(int abc) { abc ^ ++; }", // whitespace - "void f(int abc) { ^abc^++; }", // range of identifier - "void f(int abc) { ++^abc^; }", // range of identifier - "void f(int abc) { ++^abc; }", // range of identifier - "void f(int abc) { ^+^+abc; }", // range of operator - "void f(int abc) { ^abc^ ++; }", // range of identifier - "void f(int abc) { abc ^++^; }", // range of operator - "void f(int abc) { ^++^ abc; }", // range of operator - "void f(int abc) { ++ ^abc^; }", // range of identifier - "void f(int abc) { ^++^/**/abc; }", // range of operator - "void f(int abc) { ++/**/^abc; }", // range of identifier - "void f(int abc) { ^abc^/**/++; }", // range of identifier - "void f(int abc) { abc/**/^++; }", // range of operator - "void f() {^ }", // outside of identifier and operator - "int ^λλ^λ();", // UTF-8 handled properly when backing up - - // identifier in macro arg - "MACRO(bar->^func())", // beginning of identifier - "MACRO(bar->^fun^c())", // inside identifier - "MACRO(bar->^func^())", // end of identifier - "MACRO(^bar->func())", // begin identifier - "MACRO(^bar^->func())", // end identifier - "^MACRO(bar->func())", // beginning of macro name - "^MAC^RO(bar->func())", // inside macro name - "^MACRO^(bar->func())", // end of macro name - }) { - std::string WithPreamble = Preamble + Text; - Annotations TestCase(WithPreamble); - auto AST = TestTU::withCode(TestCase.code()).build(); - const auto &SourceMgr = AST.getSourceManager(); - SourceLocation Actual = getBeginningOfIdentifier( - TestCase.points().back(), SourceMgr, AST.getLangOpts()); - Position ActualPos = offsetToPosition( - TestCase.code(), - SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual))); - EXPECT_EQ(TestCase.points().front(), ActualPos) << Text; - } -} - TEST(SourceCodeTests, CollectIdentifiers) { auto Style = format::getLLVMStyle(); auto IDs = collectIdentifiers(R"cpp( @@ -481,9 +427,11 @@ )cpp"); TestTU TU = TestTU::withCode(Code.code()); auto AST = TU.build(); - auto Loc = getBeginningOfIdentifier(Code.point(), AST.getSourceManager(), - AST.getLangOpts()); - auto Result = locateMacroAt(Loc, AST.getPreprocessor()); + auto CurLoc = sourceLocationInMainFile(AST.getSourceManager(), Code.point()); + ASSERT_TRUE(bool(CurLoc)); + const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens()); + ASSERT_TRUE(Id); + auto Result = locateMacroAt(Id->location(), AST.getPreprocessor()); ASSERT_TRUE(Result); EXPECT_THAT(*Result, MacroName("MACRO")); }