[clang-tools-extra] 2bb7774 - [clangd] Get rid of getBeginningOfIdentifier helper
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 27 00:39:24 PST 2020
Author: Kadir Cetinkaya
Date: 2020-02-27T09:34:45+01:00
New Revision: 2bb7774ddf002668816fdb7c4a5b8da322fe284e
URL: https://github.com/llvm/llvm-project/commit/2bb7774ddf002668816fdb7c4a5b8da322fe284e
DIFF: https://github.com/llvm/llvm-project/commit/2bb7774ddf002668816fdb7c4a5b8da322fe284e.diff
LOG: [clangd] Get rid of getBeginningOfIdentifier helper
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75176
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 851d742711e5..ce0c6d11cada 100644
--- 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 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
format::FormatStyle Style,
const SymbolIndex *Index) {
const SourceManager &SM = AST.getSourceManager();
- llvm::Optional<HoverInfo> 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 TokensTouchingCursor =
+ syntax::spelledTokensTouching(*CurLoc, AST.getTokens());
+ if (TokensTouchingCursor.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 IdentLoc;
+ SourceLocation AutoLoc;
+ for (const auto &Tok : TokensTouchingCursor) {
+ if (Tok.kind() == tok::identifier)
+ IdentLoc = Tok.location();
+ if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype)
+ AutoLoc = Tok.location();
+ }
+
+ llvm::Optional<HoverInfo> HI;
+ if (auto Deduced = getDeducedType(AST.getASTContext(), AutoLoc)) {
HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
- } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
+ HI->SymRange =
+ getTokenRange(AST.getSourceManager(), AST.getLangOpts(), AutoLoc);
+ } else if (auto M = locateMacroAt(IdentLoc, AST.getPreprocessor())) {
HI = getHoverContents(*M, AST);
+ HI->SymRange =
+ getTokenRange(AST.getSourceManager(), AST.getLangOpts(), IdentLoc);
} 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<const Decl *> Result;
if (const SelectionTree::Node *N = ST.commonAncestor()) {
auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
@@ -565,9 +587,15 @@ 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 = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
- SourceLocationBeg);
return HI;
}
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 5ac738055b54..2b0a857228bb 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -235,108 +235,6 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM,
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/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 112f217df134..137a6f664ed9 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -78,14 +78,6 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM,
llvm::Expected<SourceLocation> 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/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
index 5ecdbeb0e308..963101e2d219 100644
--- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -87,9 +87,9 @@ TEST(CollectMainFileMacros, SelectedMacros) {
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
index 57b7c884277c..305e07d7c0c3 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -312,60 +312,6 @@ TEST(SourceCodeTests, SourceLocationInMainFile) {
}
}
-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<std::string>{
- "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 @@ TEST(SourceCodeTests, GetMacros) {
)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"));
}
More information about the cfe-commits
mailing list