[clang-tools-extra] a6860c1 - [clangd] Add a flag for spelled references in the Index

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 5 23:21:40 PST 2020


Author: Kirill Bobyrev
Date: 2020-02-06T08:18:14+01:00
New Revision: a6860c1af45776e349eaed3e8f0bb7e97abccd89

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

LOG: [clangd] Add a flag for spelled references in the Index

This patch allows the index does to provide a way to distinguish
implicit references (e.g. coming from macro expansions) from the spelled
ones. The corresponding flag was added to RefKind and symbols that are
referenced without spelling their name explicitly are now marked
implicit. This allows fixing incorrect behavior when renaming a symbol
that was referenced in macro expansions would try to rename macro
invocations.

Differential Revision: D72746

Reviewed by: hokein

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/Ref.h
    clang-tools-extra/clangd/index/SymbolCollector.cpp
    clang-tools-extra/clangd/unittests/RenameTests.cpp
    clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/Ref.h b/clang-tools-extra/clangd/index/Ref.h
index 4d6ae169693e..382ade712017 100644
--- a/clang-tools-extra/clangd/index/Ref.h
+++ b/clang-tools-extra/clangd/index/Ref.h
@@ -27,10 +27,43 @@ namespace clangd {
 /// This is a bitfield which can be combined from 
diff erent 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,
+  // Points to symbol declaration. Example:
+  //
+  // class Foo;
+  //       ^ Foo declaration
+  // Foo foo;
+  // ^ this does not reference Foo declaration
+  Declaration = 1 << 0,
+  // Points to symbol definition. Example:
+  //
+  // int foo();
+  //     ^ references foo declaration, but not foo definition
+  // int foo() { return 42; }
+  //     ^ references foo definition, but not declaration
+  // bool bar() { return true; }
+  //      ^ references both definition and declaration
+  Definition = 1 << 1,
+  // Points to symbol reference. Example:
+  //
+  // int Foo = 42;
+  // int Bar = Foo + 1;
+  //           ^ this is a reference to Foo
+  Reference = 1 << 2,
+  // The reference explicitly spells out declaration's name. Such references can
+  // not come from macro expansions or implicit AST nodes.
+  //
+  // class Foo { public: Foo() {} };
+  //       ^ references declaration, definition and explicitly spells out name
+  // #define MACRO Foo
+  //     v there is an implicit constructor call here which is not a spelled ref
+  // Foo foo;
+  // ^ this reference explicitly spells out Foo's name
+  // struct Bar {
+  //   MACRO Internal;
+  //   ^ this references Foo, but does not explicitly spell out its name
+  // };
+  Spelled = 1 << 3,
+  All = Declaration | Definition | Reference | Spelled,
 };
 
 inline RefKind operator|(RefKind L, RefKind R) {

diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 8718b7bcd48f..8a08ae894526 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/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"
@@ -167,8 +168,17 @@ bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) {
          isa<TagDecl>(&ND) && !isInsideMainFile(ND.getLocation(), SM);
 }
 
-RefKind toRefKind(index::SymbolRoleSet Roles) {
-  return static_cast<RefKind>(static_cast<unsigned>(RefKind::All) & Roles);
+RefKind toRefKind(index::SymbolRoleSet Roles, bool Spelled = false) {
+  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 (Spelled)
+    Result |= RefKind::Spelled;
+  return Result;
 }
 
 bool shouldIndexRelation(const index::SymbolRelation &R) {
@@ -279,7 +289,7 @@ bool SymbolCollector::handleDeclOccurrence(
   // occurrence inside the base-specifier.
   processRelations(*ND, *ID, Relations);
 
-  bool CollectRef = static_cast<unsigned>(Opts.RefFilter) & Roles;
+  bool CollectRef = static_cast<bool>(Opts.RefFilter & toRefKind(Roles));
   bool IsOnlyRef =
       !(Roles & (static_cast<unsigned>(index::SymbolRole::Declaration) |
                  static_cast<unsigned>(index::SymbolRole::Definition)));
@@ -544,7 +554,8 @@ void SymbolCollector::finish() {
   };
   auto CollectRef =
       [&](SymbolID ID,
-          const std::pair<SourceLocation, index::SymbolRoleSet> &LocAndRole) {
+          const std::pair<SourceLocation, index::SymbolRoleSet> &LocAndRole,
+          bool Spelled = false) {
         auto FileID = SM.getFileID(LocAndRole.first);
         // FIXME: use the result to filter out references.
         shouldIndexFile(FileID);
@@ -555,21 +566,35 @@ void SymbolCollector::finish() {
           R.Location.Start = Range.first;
           R.Location.End = Range.second;
           R.Location.FileURI = FileURI->c_str();
-          R.Kind = toRefKind(LocAndRole.second);
+          R.Kind = toRefKind(LocAndRole.second, Spelled);
           Refs.insert(ID, R);
         }
       };
   // Populate Refs slab from MacroRefs.
-  for (const auto &IDAndRefs : MacroRefs) {
+  // FIXME: All MacroRefs are marked as Spelled now, but this should be checked.
+  for (const auto &IDAndRefs : MacroRefs)
     for (const auto &LocAndRole : IDAndRefs.second)
       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)
-          CollectRef(*ID, LocAndRole);
+  llvm::DenseMap<FileID, std::vector<syntax::Token>> FilesToTokensCache;
+  for (auto &DeclAndRef : DeclRefs) {
+    if (auto ID = getSymbolID(DeclAndRef.first)) {
+      for (auto &LocAndRole : DeclAndRef.second) {
+        const auto FileID = SM.getFileID(LocAndRole.first);
+        // FIXME: It's better to use TokenBuffer by passing spelled tokens from
+        // the caller of SymbolCollector.
+        if (!FilesToTokensCache.count(FileID))
+          FilesToTokensCache[FileID] =
+              syntax::tokenize(FileID, SM, ASTCtx->getLangOpts());
+        llvm::ArrayRef<syntax::Token> Tokens = FilesToTokensCache[FileID];
+        // Check if the referenced symbol is spelled exactly the same way the
+        // corresponding NamedDecl is. If it is, mark this reference as spelled.
+        const auto *IdentifierToken =
+            spelledIdentifierTouching(LocAndRole.first, Tokens);
+        DeclarationName Name = DeclAndRef.first->getDeclName();
+        bool Spelled = IdentifierToken && Name.isIdentifier() &&
+                       Name.getAsString() == IdentifierToken->text(SM);
+        CollectRef(*ID, LocAndRole, Spelled);
       }
     }
   }

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index cb7911a4d94d..94f78ac0d977 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -861,7 +861,7 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) {
       {
           // variables.
           R"cpp(
-      static const int [[VA^R]] = 123;
+        static const int [[VA^R]] = 123;
       )cpp",
           R"cpp(
         #include "foo.h"

diff  --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index b3757d92ac93..773260c883c0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -700,6 +700,41 @@ TEST_F(SymbolCollectorTest, MacrosWithRefFilter) {
   EXPECT_THAT(Refs, IsEmpty());
 }
 
+TEST_F(SymbolCollectorTest, SpelledReference) {
+  Annotations Header(R"cpp(
+  struct Foo;
+  #define MACRO Foo
+  )cpp");
+  Annotations Main(R"cpp(
+  struct $spelled[[Foo]] {
+    $spelled[[Foo]]();
+    ~$spelled[[Foo]]();
+  };
+  $spelled[[Foo]] Variable1;
+  $implicit[[MACRO]] Variable2;
+  )cpp");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = false;
+  runSymbolCollector(Header.code(), Main.code());
+  const auto SpelledRanges = Main.ranges("spelled");
+  const auto ImplicitRanges = Main.ranges("implicit");
+  RefSlab::Builder SpelledSlabBuilder, ImplicitSlabBuilder;
+  for (const auto &SymbolAndRefs : Refs) {
+    const auto Symbol = SymbolAndRefs.first;
+    for (const auto &Ref : SymbolAndRefs.second)
+      if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
+        SpelledSlabBuilder.insert(Symbol, Ref);
+      else
+        ImplicitSlabBuilder.insert(Symbol, Ref);
+  }
+  const auto SpelledRefs = std::move(SpelledSlabBuilder).build(),
+             ImplicitRefs = std::move(ImplicitSlabBuilder).build();
+  EXPECT_THAT(SpelledRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+                                         HaveRanges(SpelledRanges))));
+  EXPECT_THAT(ImplicitRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+                                          HaveRanges(ImplicitRanges))));
+}
+
 TEST_F(SymbolCollectorTest, NameReferences) {
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;


        


More information about the cfe-commits mailing list