kbobyrev updated this revision to Diff 239502.
kbobyrev added a comment.

Actually, this approach might be more generic (i.e. not relying on
implementation too much). Added the FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72746/new/

https://reviews.llvm.org/D72746

Files:
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp

Index: clang/lib/Tooling/Syntax/Tokens.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -256,21 +256,27 @@
 
 llvm::ArrayRef<syntax::Token>
 syntax::spelledTokensTouching(SourceLocation Loc,
-                              const syntax::TokenBuffer &Tokens) {
+                              llvm::ArrayRef<syntax::Token> Tokens) {
   assert(Loc.isFileID());
-  llvm::ArrayRef<syntax::Token> All =
-      Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
   auto *Right = llvm::partition_point(
-      All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
-  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
-  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+      Tokens, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != Tokens.end() && Right->location() <= Loc;
+  bool AcceptLeft =
+      Right != Tokens.begin() && (Right - 1)->endLocation() >= Loc;
   return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
                             Right + (AcceptRight ? 1 : 0));
 }
 
+llvm::ArrayRef<syntax::Token>
+syntax::spelledTokensTouching(SourceLocation Loc,
+                              const syntax::TokenBuffer &Tokens) {
+  return spelledTokensTouching(
+      Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 const syntax::Token *
 syntax::spelledIdentifierTouching(SourceLocation Loc,
-                                  const syntax::TokenBuffer &Tokens) {
+                                  llvm::ArrayRef<syntax::Token> Tokens) {
   for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
     if (Tok.kind() == tok::identifier)
       return &Tok;
@@ -278,6 +284,13 @@
   return nullptr;
 }
 
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+                                  const syntax::TokenBuffer &Tokens) {
+  return spelledIdentifierTouching(
+      Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 std::vector<const syntax::Token *>
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===================================================================
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -313,11 +313,22 @@
   const SourceManager *SourceMgr;
 };
 
+/// The spelled tokens that overlap or touch a spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef<syntax::Token>
+spelledTokensTouching(SourceLocation Loc, llvm::ArrayRef<syntax::Token> Tokens);
+
 /// The spelled tokens that overlap or touch a spelling location Loc.
 /// This always returns 0-2 tokens.
 llvm::ArrayRef<syntax::Token>
 spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
 
+/// The identifier token that overlaps or touches a spelling location Loc.
+/// If there is none, returns nullptr.
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+                          llvm::ArrayRef<syntax::Token> Tokens);
+
 /// The identifier token that overlaps or touches a spelling location Loc.
 /// If there is none, returns nullptr.
 const syntax::Token *
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -33,7 +33,7 @@
 // Convert a Range to a Ref.
 Ref refWithRange(const clangd::Range &Range, const std::string &URI) {
   Ref Result;
-  Result.Kind = RefKind::Reference;
+  Result.Kind = RefKind::Reference | RefKind::Spelled;
   Result.Location.Start.setLine(Range.start.line);
   Result.Location.Start.setColumn(Range.start.character);
   Result.Location.End.setLine(Range.end.line);
@@ -728,6 +728,22 @@
     llvm::StringRef FooH;
     llvm::StringRef FooCC;
   } Cases[] = {
+      {
+          // Implicit references in macro expansions.
+          R"cpp(
+        class [[Fo^o]] {};
+        #define FooFoo Foo
+        #define FOO Foo
+      )cpp",
+          R"cpp(
+        #include "foo.h"
+        void bar() {
+          [[Foo]] x;
+          FOO y;
+          FooFoo z;
+        }
+      )cpp",
+      },
       {
           // classes.
           R"cpp(
@@ -837,7 +853,7 @@
       {
           // variables.
           R"cpp(
-      static const int [[VA^R]] = 123;
+        static const int [[VA^R]] = 123;
       )cpp",
           R"cpp(
         #include "foo.h"
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -296,6 +296,8 @@
   bool HasMore = Index.refs(RQuest, [&](const Ref &R) {
     if (AffectedFiles.size() > MaxLimitFiles)
       return;
+    if (!static_cast<unsigned>(R.Kind & RefKind::Spelled))
+      return;
     if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
       if (*RefFilePath != MainFile)
         AffectedFiles[*RefFilePath].push_back(toRange(R.Location));
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -19,6 +19,7 @@
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Regex.h"
 #include <functional>
@@ -169,6 +170,7 @@
   // Cache whether to index a file or not.
   llvm::DenseMap<FileID, bool> FilesToIndexCache;
   llvm::DenseMap<FileID, bool> HeaderIsSelfContainedCache;
+  llvm::DenseMap<FileID, std::vector<syntax::Token>> FilesToTokensCache;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -28,6 +28,7 @@
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -180,7 +181,16 @@
 }
 
 RefKind toRefKind(index::SymbolRoleSet Roles) {
-  return static_cast<RefKind>(static_cast<unsigned>(RefKind::All) & Roles);
+  RefKind Result = RefKind::Unknown;
+  if (Roles & static_cast<unsigned>(index::SymbolRole::Declaration))
+    Result |= RefKind::Declaration;
+  if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
+    Result |= RefKind::Definition;
+  if (Roles & static_cast<unsigned>(index::SymbolRole::Reference))
+    Result |= RefKind::Reference;
+  if (!(Roles & static_cast<unsigned>(index::SymbolRole::Implicit)))
+    Result |= RefKind::Spelled;
+  return Result;
 }
 
 bool shouldIndexRelation(const index::SymbolRelation &R) {
@@ -291,7 +301,7 @@
   // occurrence inside the base-specifier.
   processRelations(*ND, *ID, Relations);
 
-  bool CollectRef = static_cast<unsigned>(Opts.RefFilter) & Roles;
+  bool CollectRef = static_cast<unsigned>(Opts.RefFilter & toRefKind(Roles));
   bool IsOnlyRef =
       !(Roles & (static_cast<unsigned>(index::SymbolRole::Declaration) |
                  static_cast<unsigned>(index::SymbolRole::Definition)));
@@ -577,11 +587,32 @@
       CollectRef(IDAndRefs.first, LocAndRole);
   }
   // Populate Refs slab from DeclRefs.
-  if (auto MainFileURI = GetURI(SM.getMainFileID())) {
-    for (const auto &It : DeclRefs) {
-      if (auto ID = getSymbolID(It.first)) {
-        for (const auto &LocAndRole : It.second)
+  const auto MainFileID = SM.getMainFileID();
+  if (const auto MainFileURI = GetURI(MainFileID)) {
+    assert(ASTCtx && "ASTContext must be set.");
+    // FIXME: It's better to use TokenBuffer by passing spelled tokens from the
+    // caller of SymbolCollector.
+    if (!FilesToTokensCache.count(MainFileID))
+      FilesToTokensCache[MainFileID] =
+          syntax::tokenize(MainFileID, SM, ASTCtx->getLangOpts());
+    const auto Tokens = FilesToTokensCache[MainFileID];
+    for (auto &DeclAndRef : DeclRefs) {
+      if (auto ID = getSymbolID(DeclAndRef.first)) {
+        for (auto &LocAndRole : DeclAndRef.second) {
+          // Check if the referenced symbol is spelled exactly the same way the
+          // corresponding NamedDecl is. If it isn't, mark this reference as
+          // implicit.  An example of implicit references would be a macro
+          // expansion.
+          const auto *IdentifierToken =
+              spelledIdentifierTouching(LocAndRole.first, Tokens);
+          DeclarationName Name = DeclAndRef.first->getDeclName();
+          if (!IdentifierToken ||
+              (Name.isIdentifier() &&
+               Name.getAsString() != IdentifierToken->text(SM)))
+            LocAndRole.second |=
+                static_cast<uint32_t>(index::SymbolRole::Implicit);
           CollectRef(*ID, LocAndRole);
+        }
       }
     }
   }
Index: clang-tools-extra/clangd/index/Ref.h
===================================================================
--- clang-tools-extra/clangd/index/Ref.h
+++ clang-tools-extra/clangd/index/Ref.h
@@ -27,10 +27,11 @@
 /// This is a bitfield which can be combined from different kinds.
 enum class RefKind : uint8_t {
   Unknown = 0,
-  Declaration = static_cast<uint8_t>(index::SymbolRole::Declaration),
-  Definition = static_cast<uint8_t>(index::SymbolRole::Definition),
-  Reference = static_cast<uint8_t>(index::SymbolRole::Reference),
-  All = Declaration | Definition | Reference,
+  Declaration = 1 << 0,
+  Definition = 1 << 1,
+  Reference = 1 << 2,
+  Spelled = 1 << 3,
+  All = Declaration | Definition | Reference | Spelled,
 };
 
 inline RefKind operator|(RefKind L, RefKind R) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to