[clangd] Add xref for macro to static index.

[usaxena95]

Summary

This adds the references for macros to the SymbolCollector (used for static index).

After this patch:
Num refs increases by 5.7%
Disk usage increases by: 2.5%

[merge_guards_bot]

Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.
Log files: console-log.txt, CMakeCache.txt

[hokein]

clang-tools-extra/clangd/index/SymbolCollector.cpp

393

I think we should avoid running the code below if this is a mere reference.

561

I'm curious of the behavior getMacroDefinition -- from its implementation, if II doesn't have macro definition, it just returns empty.

could you check whether it works at the below case (or even with a same macro name defined/undef multiple times)?

#define abc 1
#undef abc

// not at the EOF, I guess, II for `abc` doesn't have macro definition at this point because it has been undef?

clang-tools-extra/clangd/index/SymbolCollector.h

76

This option is for macro symbols, I'd not use it for collecting macro references. I think the whether to collect macro references is judged by RefFilter and RefsInHeaders.

154

since we now use it for macros, the name is not valid any more, maybe rename it to SymbolRef?

[usaxena95]

Addressed comments.

[usaxena95]

clang-tools-extra/clangd/index/SymbolCollector.cpp

393

Interesting. Just to understand it better can you give an example where the Role is just a reference ?

561

I had a FIXME in the tests for this case. I see why there was no macro definition at that point. I guess it would be better to keep the symbol id instead of the II.

clang-tools-extra/clangd/index/SymbolCollector.h

76

I see. RefFilter sounds right but I am don't think RefsInHeaders is the correct one. It just tells whether to collect references from the included header (i.e. outside mainfile) or not.
I think it would be better to have a separate flag for macro references in such case.
WDYT ?

[merge_guards_bot]

Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.

Log files: console-log.txt, CMakeCache.txt

[usaxena95]

clang-tools-extra/clangd/index/SymbolCollector.cpp

393

My bad. Got this one. The part below it is just for the symbol slab so it's fine to omit a reference.

[usaxena95]

Added correct documentation.

[merge_guards_bot]

Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.

Log files: console-log.txt, CMakeCache.txt

[hokein]

btw, could you measure the increasing size of the index with this patch?

You could run

./bin/clangd-indexer -format=binary -executor=all-TUs . > static-index.idx
./bin/dexp static-index.idx
# you will see the memory usage.

clang-tools-extra/clangd/index/SymbolCollector.cpp

561

sorry, I didn't see a FIXME in the test, am I missing anything? Maybe move the FIXME to here?

clang-tools-extra/clangd/index/SymbolCollector.h

76

I don't think we need an extra flag merely for macro references. macro references should be treated in the same way of decl references.

[usaxena95]

Addressed comments.
Will update the increase in size of index shortly.

clang-tools-extra/clangd/index/SymbolCollector.cpp

561

Instead of storing the II, I now store the SymbolID. So we do not need to call getMacroDefinition at the end of TU.
Therefore the problem with undef and multiple declaration of macros was resolved and I removed the FIXME too.
There is a test of the form

// Macro defined multiple times.
#define $ud1[[UD]] 1
int ud_1 = $ud1[[UD]];
#undef UD

#define $ud2[[UD]] 2
int ud_2 = $ud2[[UD]];
#undef UD

clang-tools-extra/clangd/index/SymbolCollector.h

76

Got it. Done.

[merge_guards_bot]

Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.

Log files: console-log.txt, CMakeCache.txt

[usaxena95]

Before this patch
CollectMacro is disabled by default

Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes
  - number of symbols: 462463
  - number of refs: 6361763
  - numnber of relations: 20322

Before this patch
CollectMacros=true enabled in IndexerMain.cpp
Num symbols increases by 3%
Disk increase: 1.2%

Loaded Dex from no-macro-xref.idx with estimated memory usage 358596403 bytes
  - number of symbols: 476056
  - number of refs: 6361764
  - numnber of relations: 20322

After this patch
CollectMacros=true enabled in IndexerMain.cpp
Num symbols increases by 3%
Num refs increases by 5.7%
Disk increase: 3.7%

Loaded Dex from with-macro-xref.idx with estimated memory usage 367648065 bytes
  - number of symbols: 476069
  - number of refs: 6727042
  - numnber of relations: 20324

[hokein]

thanks, mostly good, my main concern is that the patch still relies on the CollectMacro and CollectMainFileSymbols option, see my comments below.

clang-tools-extra/clangd/index/SymbolCollector.cpp

348–349

again, CollectMacro just tells whether we should collect macro symbols, we use RefFilter to control whether we collect macro references. I think we'd need to move this code after the code of collecting macro refs.

359–360

and this as well.

472

this code should not be modified, since we don't use the CollectMacro option.

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

616

I think we don't need these flags if we change the symbolcollector as my other comments mention.

[usaxena95]

Addressed comments.

[usaxena95]

Explicitly turn off CollectMacro in test.

#Updatin D70489: [clangd] Add xref for macro to static index.

[merge_guards_bot]

Build result: fail - 60403 tests passed, 1 failed and 726 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[merge_guards_bot]

Build result: fail - 60403 tests passed, 1 failed and 726 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[hokein]

clang-tools-extra/clangd/index/SymbolCollector.cpp

472

this comment seems undone yet.

clang-tools-extra/clangd/indexer/IndexerMain.cpp

45 ↗

(On Diff #232079)

I think this change is unintended?

[usaxena95]

Addressed comments.

[usaxena95]

clang-tools-extra/clangd/index/SymbolCollector.cpp

472

I moved the Collecting references for Macros out of the if. This part of code just seems to be moved.
It's now moved to the original place.

[usaxena95]

Removed unintended space.

[merge_guards_bot]

Build result: fail - 60403 tests passed, 1 failed and 726 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[merge_guards_bot]

Build result: fail - 60403 tests passed, 1 failed and 726 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[hokein]

thanks, looks good with a few nits. I think the benchmark data doesn't correct any more with the latest patch, we don't increase number of symbols.

clang-tools-extra/clangd/index/SymbolCollector.cpp

368

nit: symbols => macros, and other places as well.

552

nit: I'd just inline the ID here.

no {} needed for single-body for statement.

559

nit: remove the {}.

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

672

could you add a comment why we use _ here rather than the macro name?

701

nit: this variable is not used.

[usaxena95]

Addressed comments.

[merge_guards_bot]

Build result: fail - 60403 tests passed, 1 failed and 726 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[usaxena95]

Updated benchmarks:
Before this patch

Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes
  - number of symbols: 462463
  - number of refs: 6361763
  - numnber of relations: 20322

After this patch
Num refs increases by 5.7%
Disk increase: 2.5%

Loaded Dex from with-macro-xref.idx with estimated memory usage 363390426 bytes
  - number of symbols: 462482
  - number of refs: 6727051
  - numnber of relations: 20325

clang-tools-extra/clangd/index/SymbolCollector.h

struct Options {
     /// This flag is only meaningful when RefFilter is set.
     bool RefsInHeaders = false;
     // Every symbol collected will be stamped with this origin.
     SymbolOrigin Origin = SymbolOrigin::Unknown;
     /// Collect macros.
     /// Note that SymbolCollector must be run with preprocessor in order to
     /// collect macros. For example, `indexTopLevelDecls` will not index any
     /// macro even if this is true.
     bool CollectMacro = false;
     /// Collect symbols local to main-files, such as static functions
     /// and symbols inside an anonymous namespace.
     bool CollectMainFileSymbols = true;
     /// If set to true, SymbolCollector will collect doc for all symbols.
     /// Note that documents of symbols being indexed for completion will always
     /// be collected regardless of this option.
     bool StoreAllDocumentation = false;
     /// If this is set, only collect symbols/references from a file if
private:
   RefSlab::Builder Refs;
   // All relations collected from the AST.
   RelationSlab::Builder Relations;
   ASTContext *ASTCtx;
   std::shared_ptr<Preprocessor> PP;
   std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
   std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
   Options Opts;
-  using DeclRef = std::pair<SourceLocation, index::SymbolRoleSet>;
+  using SymbolRef = std::pair<SourceLocation, index::SymbolRoleSet>;
   // Symbols referenced from the current TU, flushed on finish().
   llvm::DenseSet<const NamedDecl *> ReferencedDecls;
   llvm::DenseSet<const IdentifierInfo *> ReferencedMacros;
-  llvm::DenseMap<const NamedDecl *, std::vector<DeclRef>> DeclRefs;
+  llvm::DenseMap<const NamedDecl *, std::vector<SymbolRef>> DeclRefs;
+  llvm::DenseMap<SymbolID, std::vector<SymbolRef>> MacroRefs;
   // Maps canonical declaration provided by clang to canonical declaration for
   // an index symbol, if clangd prefers a different declaration than that
   // provided by clang. For example, friend declaration might be considered
   // canonical by clang but should not be considered canonical in the index
   // unless it's a definition.
   llvm::DenseMap<const Decl *, const Decl *> CanonicalDecls;
   // Cache whether to index a file or not.
   llvm::DenseMap<FileID, bool> FilesToIndexCache;
   llvm::DenseMap<FileID, bool> HeaderIsSelfContainedCache;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/index/SymbolCollector.cpp

 #include "CanonicalIncludes.h"
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
 #include "ExpectedTypes.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "SymbolLocation.h"
 #include "URI.h"
+#include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
     addDefinition(*OriginalDecl, *BasicSymbol);
 
   return true;
 }
 
 bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
                                            const MacroInfo *MI,
                                            index::SymbolRoleSet Roles,
                                            SourceLocation Loc) {
-  if (!Opts.CollectMacro)
-    return true;
   assert(PP.get());
 
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
+  auto SpellingLoc = SM.getSpellingLoc(Loc);
+  bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
 
   // Builtin macros don't have useful locations and aren't needed in completion.
   if (MI->isBuiltinMacro())
     return true;
 
-  // Skip main-file symbols if we are not collecting them.
-  bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
-  if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
-    return false;
-
   // Also avoid storing predefined macros like __DBL_MIN__.
   if (SM.isWrittenInBuiltinFile(DefLoc))
     return true;
 
+  auto ID = getSymbolID(Name->getName(), MI, SM);
+  if (!ID)
+    return true;
+
+  // Do not store references to main-file symbols.
+  if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileSymbol &&
+      (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
+    MacroRefs[*ID].push_back({Loc, Roles});
+
+  // Collect symbols.
+  if (!Opts.CollectMacro)
+    return true;
+
+  // Skip main-file symbols if we are not collecting them.
+  if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
+    return false;
+
   // Mark the macro as referenced if this is a reference coming from the main
   // file. The macro may not be an interesting symbol, but it's cheaper to check
   // at the end.
   if (Opts.CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
-      SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
+      SM.getFileID(SpellingLoc) == SM.getMainFileID())
     ReferencedMacros.insert(Name);
+
   // Don't continue indexing if this is a mere reference.
   // FIXME: remove macro with ID if it is undefined.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
     return true;
 
-  auto ID = getSymbolID(Name->getName(), MI, SM);
-  if (!ID)
-    return true;
-
   // Only collect one instance in case there are multiple.
   if (Symbols.find(*ID) != nullptr)
     return true;
 
   Symbol S;
   S.ID = std::move(*ID);
   S.Name = Name->getName();
   if (!IsMainFileSymbol) {
if (const auto *S = Symbols.find(ID)) {
       Symbols.insert(Inc);
     }
   };
   for (const NamedDecl *ND : ReferencedDecls) {
     if (auto ID = getSymbolID(ND)) {
       IncRef(*ID);
     }
   }
   if (Opts.CollectMacro) {
     assert(PP);
     // First, drop header guards. We can't identify these until EOF.
     for (const IdentifierInfo *II : IndexedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
           if (MI->isUsedForHeaderGuard())
             Symbols.erase(*ID);
     }
     // Now increment refcounts.
     for (const IdentifierInfo *II : ReferencedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
           IncRef(*ID);
     }
   }
-
   // Fill in IncludeHeaders.
   // We delay this until end of TU so header guards are all resolved.
-  // Symbols in slabs aren' mutable, so insert() has to walk all the strings :-(
+  // Symbols in slabs aren' mutable, so insert() has to walk all the strings
+  // :-(
   llvm::SmallString<256> QName;
   for (const auto &Entry : IncludeFiles)
     if (const Symbol *S = Symbols.find(Entry.first)) {
       QName = S->Scope;
       QName.append(S->Name);
       if (auto Header = getIncludeHeader(QName, Entry.second)) {
         Symbol NewSym = *S;
         NewSym.IncludeHeaders.push_back({*Header, 1});
if (Found == URICache.end()) {
         // Ignore cases where we can not find a corresponding file entry
         // for the loc, thoses are not interesting, e.g. symbols formed
         // via macro concatenation.
         return None;
       }
     }
     return Found->second;
   };
-  // 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) {
-          auto FileID = SM.getFileID(LocAndRole.first);
-          // FIXME: use the result to filter out references.
-          shouldIndexFile(FileID);
-          if (auto FileURI = GetURI(FileID)) {
-            auto Range =
-                getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-            Ref R;
-            R.Location.Start = Range.first;
-            R.Location.End = Range.second;
-            R.Location.FileURI = FileURI->c_str();
-            R.Kind = toRefKind(LocAndRole.second);
-            Refs.insert(*ID, R);
-          }
+  auto CollectRef =
+      [&](SymbolID ID,
+          const std::pair<SourceLocation, index::SymbolRoleSet> &LocAndRole) {
+          Refs.insert(ID, R);
+        }
+      };
+  // Populate Refs slab from MacroRefs.
+  for (const auto &MacroRef : MacroRefs) {
+    const SymbolID &ID = MacroRef.first;
+    for (const auto &LocAndRole : MacroRef.second) {
+      CollectRef(ID, 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);
         }
       }
     }
   }
 
   ReferencedDecls.clear();
   ReferencedMacros.clear();
   DeclRefs.clear();
   FilesToIndexCache.clear();

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

 namespace {
 
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedElementsAreArray;
 
 // GMock helpers for matching Symbol.
 MATCHER_P(Labeled, Label, "") {
   return (arg.Name + arg.Signature).str() == Label;
public:
                   CommentHandler *PragmaHandler)
           : DataConsumer(std::move(DataConsumer)), Opts(Opts),
             PragmaHandler(PragmaHandler) {}
 
       std::unique_ptr<ASTConsumer>
       CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
         if (PragmaHandler)
           CI.getPreprocessor().addCommentHandler(PragmaHandler);
-        return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr());
+        return createIndexingASTConsumer(DataConsumer, Opts,
+                                         CI.getPreprocessorPtr());
       }
 
       bool BeginInvocation(CompilerInstance &CI) override {
         // Make the compiler parse all comments.
         CI.getLangOpts().CommentOpts.ParseAllComments = true;
         return true;
       }
 
EXPECT_THAT(Symbols,
                         DefRange(Main.range("printdef"))),
                   AllOf(QName("Z"), DeclRange(Header.range("zdecl"))),
                   AllOf(QName("foo"), DeclRange(Header.range("foodecl"))),
                   AllOf(QName("Y"), DeclRange(Main.range("ydecl")))));
 }
 
 TEST_F(SymbolCollectorTest, Refs) {
   Annotations Header(R"(
-  class $foo[[Foo]] {
+  #define MACRO(X) (X + 1)
+  class Foo {
   public:
-    $foo[[Foo]]() {}
-    $foo[[Foo]](int);
+    Foo() {}
+    Foo(int);
   };
-  class $bar[[Bar]];
-  void $func[[func]]();
+  class Bar;
+  void func();
 
-  namespace $ns[[NS]] {} // namespace ref is ignored
+  namespace NS {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
 
   void $func[[func]]();
 
   void fff() {
     $foo[[Foo]] foo;
     $bar[[Bar]] bar;
     $func[[func]]();
     int abc = 0;
     $foo[[Foo]] foo2 = abc;
+    abc = $macro[[MACRO]](1);
   }
   )");
   Annotations SymbolsOnlyInMainCode(R"(
+  #define FUNC(X) (X+1)
   int a;
   void b() {}
-  static const int c = 0;
+  static const int c = FUNC(1);
   class d {};
   )");
   CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.CollectMacro = true;
   runSymbolCollector(Header.code(),
                      (Main.code() + SymbolsOnlyInMainCode.code()).str());
-  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
-
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
                                   HaveRanges(Main.ranges("foo")))));
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Bar").ID,
                                   HaveRanges(Main.ranges("bar")))));
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
                                   HaveRanges(Main.ranges("func")))));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _))));
-  // Symbols *only* in the main file (a, b, c) had no refs collected.
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
+                                  HaveRanges(Main.ranges("macro")))));
+  // Symbols *only* in the main file (a, b, c, FUNC) had no refs collected.
   auto MainSymbols =
       TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _))));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _))));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _))));
+}
+
+TEST_F(SymbolCollectorTest, MacroRefInHeader) {
+  Annotations Header(R"(
+  #define $foo[[FOO]](X) (X + 1)
+  #define $bar[[BAR]](X) (X + 2)
+
+  // Macro defined multiple times.
+  #define $ud1[[UD]] 1
+  int ud_1 = $ud1[[UD]];
+  #undef UD
+
+  #define $ud2[[UD]] 2
+  int ud_2 = $ud2[[UD]];
+  #undef UD
+
+  // Macros from token concatenations not included.
+  #define $concat[[CONCAT]](X) X##A()
+  #define $prepend[[PREPEND]](X) MACRO##X()
+  #define $macroa[[MACROA]]() 123
+  int B = $concat[[CONCAT]](MACRO);
+  int D = $prepend[[PREPEND]](A);
+
+  void fff() {
+    int abc = $foo[[FOO]](1) + $bar[[BAR]]($foo[[FOO]](1));
+  }
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  // Need this to get the SymbolID for macros for tests.
+  CollectorOpts.CollectMacro = true;
+
+  runSymbolCollector(Header.code(), "");
+
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "FOO").ID,
+                                  HaveRanges(Header.ranges("foo")))));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "BAR").ID,
+                                  HaveRanges(Header.ranges("bar")))));
+  EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud1")))));
+  EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud2")))));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "CONCAT").ID,
+                                  HaveRanges(Header.ranges("concat")))));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PREPEND").ID,
+                                  HaveRanges(Header.ranges("prepend")))));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACROA").ID,
+                                  HaveRanges(Header.ranges("macroa")))));
+}
+
+TEST_F(SymbolCollectorTest, MacroRefWithoutCollectingSymbol) {
+  Annotations Header(R"(
+  #define $foo[[FOO]](X) (X + 1)
+  int abc = $foo[[FOO]](1);
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  CollectorOpts.CollectMacro = false;
+
+  runSymbolCollector(Header.code(), "");
+
+  EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("foo")))));
+}
+
+TEST_F(SymbolCollectorTest, MacrosWithRefFilter) {
+  Annotations Header("#define $macro[[MACRO]](X) (X + 1)");
+  Annotations Main("void foo() { int x = $macro[[MACRO]](1); }");
+  CollectorOpts.RefFilter = RefKind::Unknown;
+  runSymbolCollector(Header.code(), Main.code());
+  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
+  EXPECT_THAT(Refs, IsEmpty());
 }
 
 TEST_F(SymbolCollectorTest, NameReferences) {
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   Annotations Header(R"(
     class [[Foo]] {
     public:
EXPECT_THAT(Refs,
                                    Pair(findSymbol(Symbols, "Func").ID,
                                         HaveRanges(Header.ranges("Func")))));
 
   // Run the .hh file as main file (without "-x c++-header"), we should collect
   // the refs as well.
   TestFileName = testPath("foo.hh");
   runSymbolCollector("", Header.code());
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
-  EXPECT_THAT(Refs, UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
+  EXPECT_THAT(Refs,
+              UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
                                         HaveRanges(Header.ranges("Foo"))),
                                    Pair(findSymbol(Symbols, "Func").ID,
                                         HaveRanges(Header.ranges("Func")))));
 }
 
 TEST_F(SymbolCollectorTest, RefsInHeaders) {
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
+  CollectorOpts.CollectMacro = true;
   Annotations Header(R"(
-  class [[Foo]] {};
+  #define $macro[[MACRO]](x) (x+1)
+  class $foo[[Foo]] {};
   )");
   runSymbolCollector(Header.code(), "");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
-                                  HaveRanges(Header.ranges()))));
+                                  HaveRanges(Header.ranges("foo")))));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
+                                  HaveRanges(Header.ranges("macro")))));
 }
 
 TEST_F(SymbolCollectorTest, Relations) {
   std::string Header = R"(
   class Base {};
   class Derived : public Base {};
   )";
   runSymbolCollector(Header, /*Main=*/"");
   const Symbol &Base = findSymbol(Symbols, "Base");
   const Symbol &Derived = findSymbol(Symbols, "Derived");
   EXPECT_THAT(Relations,
               Contains(Relation{Base.ID, RelationKind::BaseOf, Derived.ID}));
 }
 
-TEST_F(SymbolCollectorTest, References) {
+TEST_F(SymbolCollectorTest, CountReferences) {
   const std::string Header = R"(
     class W;
     class X {};
     class Y;
     class Z {}; // not used anywhere
     Y* y = nullptr;  // used in header doesn't count
     #define GLOBAL_Z(name) Z name;
   )";