sammccall created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple, 
ilya-biryukov, klimek.

- don't count occurrences of decls where we don't spell the name


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45356

Files:
  clangd/XRefs.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -250,14 +250,16 @@
     class Y;
     class Z {}; // not used anywhere
     Y* y = nullptr;  // used in header doesn't count
+    #define GLOBAL_Z(name) Z name;
   )";
   const std::string Main = R"(
     W* w = nullptr;
     W* w2 = nullptr; // only one usage counts
     X x();
     class V;
     V* v = nullptr; // Used, but not eligible for indexing.
     class Y{}; // definition doesn't count as a reference
+    GLOBAL_Z(z); // Not a reference to Z, we don't spell the type.
   )";
   CollectorOpts.CountReferences = true;
   runSymbolCollector(Header, Main);
Index: clangd/index/SymbolCollector.h
===================================================================
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -59,8 +59,8 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-                      ArrayRef<index::SymbolRelation> Relations, FileID FID,
-                      unsigned Offset,
+                      ArrayRef<index::SymbolRelation> Relations,
+                      SourceLocation Loc,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -225,7 +225,7 @@
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurence(
     const Decl *D, index::SymbolRoleSet Roles,
-    ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset,
+    ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
@@ -235,9 +235,10 @@
 
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
+  auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
-      ASTCtx->getSourceManager().getMainFileID() == FID)
+      SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
 
   // Don't continue indexing if this is a mere reference.
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -79,10 +79,10 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-                      ArrayRef<index::SymbolRelation> Relations, FileID FID,
-                      unsigned Offset,
+                      ArrayRef<index::SymbolRelation> Relations,
+                      SourceLocation Loc,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-    if (isSearchedLocation(FID, Offset)) {
+    if (AST.getSourceManager().getSpellingLoc(Loc) == SearchedLocation) {
       // Find and add definition declarations (for GoToDefinition).
       // We don't use parameter `D`, as Parameter `D` is the canonical
       // declaration, which is the first declaration of a redeclarable
@@ -98,12 +98,6 @@
   }
 
 private:
-  bool isSearchedLocation(FileID FID, unsigned Offset) const {
-    const SourceManager &SourceMgr = AST.getSourceManager();
-    return SourceMgr.getFileOffset(SearchedLocation) == Offset &&
-           SourceMgr.getFileID(SearchedLocation) == FID;
-  }
-
   void finish() override {
     // Also handle possible macro at the searched location.
     Token Result;
@@ -125,19 +119,8 @@
         MacroDefinition MacroDef =
             PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation);
         MacroInfo *MacroInf = MacroDef.getMacroInfo();
-        if (MacroInf) {
+        if (MacroInf)
           MacroInfos.push_back(MacroDecl{IdentifierInfo->getName(), MacroInf});
-          // Clear all collected delcarations if this is a macro search.
-          //
-          // In theory, there should be no declarataions being collected when we
-          // search a source location that refers to a macro.
-          // The occurrence location returned by `handleDeclOccurence` is
-          // limited (FID, Offset are from expansion location), we will collect
-          // all declarations inside the macro.
-          //
-          // FIXME: Avoid adding decls from inside macros in handlDeclOccurence.
-          Decls.clear();
-        }
       }
     }
   }
@@ -252,21 +235,20 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-                      ArrayRef<index::SymbolRelation> Relations, FileID FID,
-                      unsigned Offset,
+                      ArrayRef<index::SymbolRelation> Relations,
+                      SourceLocation Loc,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
     const SourceManager &SourceMgr = AST.getSourceManager();
-    if (SourceMgr.getMainFileID() != FID ||
+    SourceLocation HighlightStartLoc = SourceMgr.getFileLoc(Loc);
+    if (SourceMgr.getMainFileID() != SourceMgr.getFileID(HighlightStartLoc) ||
         std::find(Decls.begin(), Decls.end(), D) == Decls.end()) {
       return true;
     }
     SourceLocation End;
     const LangOptions &LangOpts = AST.getLangOpts();
-    SourceLocation StartOfFileLoc = SourceMgr.getLocForStartOfFile(FID);
-    SourceLocation HightlightStartLoc = StartOfFileLoc.getLocWithOffset(Offset);
     End =
-        Lexer::getLocForEndOfToken(HightlightStartLoc, 0, SourceMgr, LangOpts);
-    SourceRange SR(HightlightStartLoc, End);
+        Lexer::getLocForEndOfToken(HighlightStartLoc, 0, SourceMgr, LangOpts);
+    SourceRange SR(HighlightStartLoc, End);
 
     DocumentHighlightKind Kind = DocumentHighlightKind::Text;
     if (static_cast<index::SymbolRoleSet>(index::SymbolRole::Write) & Roles)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to