kadircet updated this revision to Diff 247439.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75331/new/
https://reviews.llvm.org/D75331
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -431,7 +431,7 @@
ASSERT_TRUE(bool(CurLoc));
const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
ASSERT_TRUE(Id);
- auto Result = locateMacroAt(Id->location(), AST.getPreprocessor());
+ auto Result = locateMacroAt(*Id, AST.getPreprocessor());
ASSERT_TRUE(Result);
EXPECT_THAT(*Result, MacroName("MACRO"));
}
@@ -445,7 +445,7 @@
ASSERT_TRUE(bool(CurLoc));
const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
ASSERT_TRUE(Id);
- auto Result = locateMacroAt(Id->location(), AST.getPreprocessor());
+ auto Result = locateMacroAt(*Id, AST.getPreprocessor());
ASSERT_TRUE(Result);
EXPECT_THAT(*Result, MacroName("MACRO"));
}
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -89,7 +89,9 @@
auto Loc = sourceLocationInMainFile(SM, ExpectedRefs.begin()->start);
ASSERT_TRUE(bool(Loc));
- auto Macro = locateMacroAt(*Loc, PP);
+ const auto *Id = syntax::spelledIdentifierTouching(*Loc, AST.getTokens());
+ ASSERT_TRUE(Id);
+ auto Macro = locateMacroAt(*Id, PP);
assert(Macro);
auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -502,7 +502,7 @@
return makeError(ReasonToReject::NoSymbolFound);
// FIXME: Renaming macros is not supported yet, the macro-handling code should
// be moved to rename tooling library.
- if (locateMacroAt(IdentifierToken->location(), AST.getPreprocessor()))
+ if (locateMacroAt(*IdentifierToken, AST.getPreprocessor()))
return makeError(ReasonToReject::UnsupportedSymbol);
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -228,8 +228,7 @@
const auto *TouchedIdentifier =
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
if (TouchedIdentifier) {
- if (auto M = locateMacroAt(TouchedIdentifier->location(),
- AST.getPreprocessor())) {
+ if (auto M = locateMacroAt(*TouchedIdentifier, AST.getPreprocessor())) {
if (auto Loc = makeLocation(AST.getASTContext(),
M->Info->getDefinitionLoc(), *MainFilePath)) {
LocatedSymbol Macro;
@@ -463,13 +462,14 @@
llvm::consumeError(CurLoc.takeError());
return {};
}
- SourceLocation SLocId;
+ llvm::Optional<DefinedMacro> Macro;
if (const auto *IdentifierAtCursor =
- syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens()))
- SLocId = IdentifierAtCursor->location();
- RefsRequest Req;
+ syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens())) {
+ Macro = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor());
+ }
- if (auto Macro = locateMacroAt(SLocId, AST.getPreprocessor())) {
+ RefsRequest Req;
+ if (Macro) {
// Handle references to macro.
if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
// Collect macro references from main file.
@@ -587,8 +587,7 @@
if (!IdentifierAtCursor)
return Results;
- if (auto M = locateMacroAt(IdentifierAtCursor->location(),
- AST.getPreprocessor())) {
+ if (auto M = locateMacroAt(*IdentifierAtCursor, AST.getPreprocessor())) {
SymbolDetails NewMacro;
NewMacro.name = std::string(M->Name);
llvm::SmallString<32> USR;
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -21,6 +21,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
@@ -281,9 +282,9 @@
llvm::StringRef Name;
const MacroInfo *Info;
};
-/// Gets the macro at a specified \p Loc. It must be a spelling location and
-/// point to the beginning of identifier.
-llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
+/// Gets the macro referenced by \p SpelledTok. It must be a spelled token
+/// aligned to the beginning of an identifier.
+llvm::Optional<DefinedMacro> locateMacroAt(const syntax::Token &SpelledTok,
Preprocessor &PP);
/// Infers whether this is a header from the FileName and LangOpts (if
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -891,17 +891,12 @@
return Result;
}
-llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
+llvm::Optional<DefinedMacro> locateMacroAt(const syntax::Token &SpelledTok,
Preprocessor &PP) {
+ SourceLocation Loc = SpelledTok.location();
assert(Loc.isFileID());
const auto &SM = PP.getSourceManager();
- const auto &LangOpts = PP.getLangOpts();
- Token Result;
- if (Lexer::getRawToken(Loc, Result, SM, LangOpts, false))
- return None;
- if (Result.is(tok::raw_identifier))
- PP.LookUpIdentifierInfo(Result);
- IdentifierInfo *IdentifierInfo = Result.getIdentifierInfo();
+ IdentifierInfo *IdentifierInfo = PP.getIdentifierInfo(SpelledTok.text(SM));
if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
return None;
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -532,32 +532,37 @@
}
auto TokensTouchingCursor =
syntax::spelledTokensTouching(*CurLoc, AST.getTokens());
+ // Early exit if there were no tokens around the cursor.
if (TokensTouchingCursor.empty())
return llvm::None;
- // 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.
+ // To be used as a backup for highlighting the selected token.
SourceLocation IdentLoc;
- SourceLocation AutoLoc;
+ 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)
+ if (Tok.kind() == tok::identifier) {
IdentLoc = Tok.location();
- if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype)
- AutoLoc = Tok.location();
+ 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());
+ break;
+ }
+ }
}
- llvm::Optional<HoverInfo> HI;
- if (auto Deduced = getDeducedType(AST.getASTContext(), AutoLoc)) {
- HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
- 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 {
+ // If it wasn't auto/decltype or macro, look for decls and expressions.
+ if (!HI) {
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).
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
