[clang-tools-extra] 3ae2fc7 - [clangd] Get rid of lexer usage in locateMacroAt

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 2 04:38:20 PST 2020


Author: Kadir Cetinkaya
Date: 2020-03-02T13:31:12+01:00
New Revision: 3ae2fc7a8bb3ee074d9fb9c0f235052b86c37aaf

URL: https://github.com/llvm/llvm-project/commit/3ae2fc7a8bb3ee074d9fb9c0f235052b86c37aaf
DIFF: https://github.com/llvm/llvm-project/commit/3ae2fc7a8bb3ee074d9fb9c0f235052b86c37aaf.diff

LOG: [clangd] Get rid of lexer usage in locateMacroAt

Reviewers: sammccall, hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75331

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index ce0c6d11cada..5796657a5800 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -532,32 +532,37 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   }
   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).

diff  --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 2c4338dce7c7..79d027def4bc 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -891,17 +891,12 @@ llvm::StringSet<> collectWords(llvm::StringRef Content) {
   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;
 

diff  --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 32fe9c6c54b3..c601cc89df28 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/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 @@ struct DefinedMacro {
   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

diff  --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index f7702c98b38f..29c2338f5bb5 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -228,8 +228,7 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
   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 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
     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 @@ std::vector<SymbolDetails> getSymbolInfo(ParsedAST &AST, Position Pos) {
   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;

diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index d31af9c2adf1..16dfef316a27 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -502,7 +502,7 @@ llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
     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());

diff  --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
index 963101e2d219..7a439c3b484b 100644
--- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -89,7 +89,9 @@ TEST(CollectMainFileMacros, SelectedMacros) {
 
       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);
 

diff  --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index 7d614785a11d..5c45675ab74d 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -431,7 +431,7 @@ TEST(SourceCodeTests, GetMacros) {
   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 @@ TEST(SourceCodeTests, WorksAtBeginOfFile) {
   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"));
 }


        


More information about the cfe-commits mailing list