[clangd] Collect symbol occurrences in SymbolCollector

[hokein]

Summary

SymbolCollector will be used for two cases:

• collect Symbol type only, used for indexing preamble AST.
• collect Symbol and SymbolOccurrences, used for indexing main AST.

For finding local references from the AST, we will implement it in other ways.

[ioeric]

2 high-level questions:

1. What's the reason for having a separate SymbolOccurrenceSlab? Could store occurrences as extra payload of Symbol?

2. Could we merge SymbolOccurrenceCollector into the existing SymbolCollector? They look a lot alike. Having another index data consumer seems like more overhead on the user side.

[hokein]

In D50385#1191914, @ioeric wrote:

2 high-level questions:

1. What's the reason for having a separate SymbolOccurrenceSlab? Could store occurrences as extra payload of Symbol?

Storing occurrences in Symbol structure is easy to misuse by users IMO -- if we go through this way, we will end up having a getOccurrences-like method in Symbol structure. Once users get the Symbol instance, it is natural for them to call getOccurrences to get all occurrences of the symbol. However this getOccurrences method doesn't do what users expected (just returning an incomplete set of results or empty). To query the symbol occurrences, we should always use index interface.

Therefore, I think we should try to avoid these confusions in the design.

2. Could we merge SymbolOccurrenceCollector into the existing SymbolCollector? They look a lot alike. Having another index data consumer seems like more overhead on the user side.

The SymbolOccurrenceCollector has many responsibilities (collecting declaration, definition, code completion information etc), and the code is growing complex now. Merging the SymbolOccurrenceCollector to it will make it more complicated -- we will introduce more option flags like collect-symbol-only, collect-occurrence-only to configure it for our different use cases (we need to the implementation detail clearly in order to make a correct option for SymbolCollector). And I can foresee these two collectors might be run at different point (runWithPreamble vs runWithAST) in dynamic index.

They might use same facilities, but we could always share them.

[ioeric]

In D50385#1193545, @hokein wrote:

In D50385#1191914, @ioeric wrote:

2 high-level questions:

1. What's the reason for having a separate SymbolOccurrenceSlab? Could store occurrences as extra payload of Symbol?

Storing occurrences in Symbol structure is easy to misuse by users IMO -- if we go through this way, we will end up having a getOccurrences-like method in Symbol structure. Once users get the Symbol instance, it is natural for them to call getOccurrences to get all occurrences of the symbol. However this getOccurrences method doesn't do what users expected (just returning an incomplete set of results or empty). To query the symbol occurrences, we should always use index interface.

Therefore, I think we should try to avoid these confusions in the design.

Hmm, I think this is the same for other symbol payload e.g. definition can be missing for a symbol. And it seems to me that the concern is on the SymbolSlab level: if a slab is for a single TU, users should expect missing information; if a slab is merged from all TUs, then users can expect "complete" information. I think it's reasonable to assume that users of SymbolSlab are aware of this. I think it's probably not worth the overhead of maintaining and using two separate slabs.

2. Could we merge SymbolOccurrenceCollector into the existing SymbolCollector? They look a lot alike. Having another index data consumer seems like more overhead on the user side.

The SymbolOccurrenceCollector has many responsibilities (collecting declaration, definition, code completion information etc), and the code is growing complex now. Merging the SymbolOccurrenceCollector to it will make it more

Although the existing SymbolCollector supports different options, I think it still has a pretty well-defined responsibility: gather information about symbols. IMO, cross-reference is one of the property of symbol, and I don't see strong reasons to keep them separated.

complicated -- we will introduce more option flags like collect-symbol-only, collect-occurrence-only to configure it for our different use cases (we need to the implementation detail clearly in order to make a correct option for SymbolCollector).

I think these options are reasonable if they turn out to be necessary. And making the SymbolCollector more complicated doesn't seem to be a problem if we are indeed doing more complicated work, but I don't think this would turn into a big problem as logic of xrefs seems pretty isolated. Conversely, I think implementing xrefs in a separate class would likely to cause more duplicate and maintenance, e.g. two sets of options, two sets of initializations or life-time tracking of collectors (they look a lot alike), the same boilerplate factory code in tests, passing around two collectors in user code.

And I can foresee these two collectors might be run at different point (runWithPreamble vs runWithAST) in dynamic index.

With some options, this should be a problem I think?

[ilya-biryukov]

In D50385#1193600, @ioeric wrote:

In D50385#1193545, @hokein wrote:

In D50385#1191914, @ioeric wrote:

2 high-level questions:

1. What's the reason for having a separate SymbolOccurrenceSlab? Could store occurrences as extra payload of Symbol?

Storing occurrences in Symbol structure is easy to misuse by users IMO -- if we go through this way, we will end up having a getOccurrences-like method in Symbol structure. Once users get the Symbol instance, it is natural for them to call getOccurrences to get all occurrences of the symbol. However this getOccurrences method doesn't do what users expected (just returning an incomplete set of results or empty). To query the symbol occurrences, we should always use index interface.

Therefore, I think we should try to avoid these confusions in the design.

Hmm, I think this is the same for other symbol payload e.g. definition can be missing for a symbol. And it seems to me that the concern is on the SymbolSlab level: if a slab is for a single TU, users should expect missing information; if a slab is merged from all TUs, then users can expect "complete" information. I think it's reasonable to assume that users of SymbolSlab are aware of this. I think it's probably not worth the overhead of maintaining and using two separate slabs.

I think it's reasonable to keep occurrences away from Symbol's Detail field. Stashing them together is only fine for the collector API, having any way to directly access occurrences through Symbol will be totally confusing for all the other users.
E.g., the Index::lookup() will not provide occurrences in the Symbol instances it returns, and if the accessors for those will be there it will only add confusion. So +1 to keeping them out of the Symbol class.

On the other hand, SymbolSlab feels like a perfectly reasonable place to store the occurrences in addition to the symbols themselves and it feels we should reuse its memory arena for storing any strings we need to allocate, etc.

2. Could we merge SymbolOccurrenceCollector into the existing SymbolCollector? They look a lot alike. Having another index data consumer seems like more overhead on the user side.

The SymbolOccurrenceCollector has many responsibilities (collecting declaration, definition, code completion information etc), and the code is growing complex now. Merging the SymbolOccurrenceCollector to it will make it more

Although the existing SymbolCollector supports different options, I think it still has a pretty well-defined responsibility: gather information about symbols. IMO, cross-reference is one of the property of symbol, and I don't see strong reasons to keep them separated.

complicated -- we will introduce more option flags like collect-symbol-only, collect-occurrence-only to configure it for our different use cases (we need to the implementation detail clearly in order to make a correct option for SymbolCollector).

I think these options are reasonable if they turn out to be necessary. And making the SymbolCollector more complicated doesn't seem to be a problem if we are indeed doing more complicated work, but I don't think this would turn into a big problem as logic of xrefs seems pretty isolated. Conversely, I think implementing xrefs in a separate class would likely to cause more duplicate and maintenance, e.g. two sets of options, two sets of initializations or life-time tracking of collectors (they look a lot alike), the same boilerplate factory code in tests, passing around two collectors in user code.

And I can foresee these two collectors might be run at different point (runWithPreamble vs runWithAST) in dynamic index.

With some options, this should be a problem I think?

+1 to merging into the SymbolCollector. Keeping the responsibilities separate inside a single class should be easy, e.g. something like that should be simple enough:

SymbolCollector::handleDeclOccurence(args) {
  this->processForSymbol(args); // handles keeping the Symbol structure up-to-date, i.e. adds definition locations, etc.
  this->processForOccurrences(args); // appends occurrences to a list of xrefs.
};

The main advantage that we get is less clang-specific boilerplate. The less IndexDataConsumers, FrontendActionFactorys, FrontendActions we create, the more focused and concise our code is.
And in that case, SymbolCollector is already handling those responsibilities for us and reusing looks like a good idea.

[hokein]

Hmm, I think this is the same for other symbol payload e.g. definition can be missing for a symbol. And it seems to me that the concern is on the SymbolSlab level: if a slab is for a single TU, users should expect missing information; if a slab is merged from all TUs, then users can expect "complete" information. I think it's reasonable to assume that users of SymbolSlab are aware of this. I think it's probably not worth the overhead of maintaining and using two separate slabs.

My concerns of storing occurrences as an extra payload of Symbol are:

• SymbolSlab is more like an implementation detail. Users of SymbolIndex are not aware of it, they only get Symbol objects, so it easily confuses users if they see any occurrence-related interface/member in Symbol. And we will write a looong comment explaining its correct behavior. It'd be better if we avoid this confusion in the API level.
• The fields in Symbol structure are symbol properties, and could be stored in memory. However, occurrences are not, we can't guarantee that.
• It seems that we are coupling ID, Symbol, SymbolOccurrence together: in the index implementation, we will go through ID=>Symbol=>Occurrences rather than ID=>Occurrences.

I think these options are reasonable if they turn out to be necessary.

I think they are necessary. For collecting all occurrences for local symbols from the AST, we only need symbol occurrence information, other information (e.g. declaration&definition location, #include) should be discarded; Index for code completion should not collect symbol occurrences.

And making the SymbolCollector more complicated doesn't seem to be a problem if we are indeed doing more complicated work, but I don't think this would turn into a big problem as logic of xrefs seems pretty isolated.

If xrefs is quite isolated, I think it is a good signal to have a dedicated class handling it.

I think implementing xrefs in a separate class would likely to cause more duplicate and maintenance, e.g. two sets of options, two sets of initializations or life-time tracking of collectors (they look a lot alike), the same boilerplate factory code in tests, passing around two collectors in user code.

Merging xrefs to SymbolCollector couldn't avoid these problems, I think it is a matter of where we put these code:

• different initialization of SymbolCollector for different use cases (e.g. setting different flags in SymbolCollectorOptions).
• for dynamic index, index for xrefs and code completion would be triggered at different point: index for xrefs should happen when AST is ready; index for code completion happens when Preamble is ready; we might end up with two slabs instances in the dynamic index (1 symbol slab + 1 occurrence slab vs. 2 symbol slabs).

The duplication is mainly about AST frontend action boilerplate code. To eliminate it, we could do some refactorings:

• get rid of the clang ast action code in SymbolCollector, and SymbolOccurrenceCollector
• introduce an IndexSymbol which is a subclass index::IndexDataConsumer
• the IndexSymbol has two mode (indexing symbol or indexing occurrence), and dispatch ast information to SymbolCollector/SymbolOccurrenceCollector.

[hokein]

Update the patch based on our offline discussion

• only one single clang intefaces implementation, and move finding references to current symbol collector;
• store references in SymbolSlab;

[ilya-biryukov]

clangd/index/Index.cpp

139

NIT: remove the lambda? using < is the default.

145

NIT: remove the lambda? Using == is the default.

158

Is this used for debugging?
In that case maybe consider having a user-readable representation instead of the number?

clangd/index/Index.h

46

NIT: having friend decls inside the classes themselves might prove to be more readable.
Not opposed to the current one too, feel free to ignore.

347

Maybe add a comment or remove the empty line?

348

Any store occurences in a file-centric manner?
E.g.

/// Occurences inside a single file.
class FileOccurences {
  StringRef File;
  vector<pair<Point, OccurenceKind>> Locations;
};

// ....
DenseMap<SymbolID, vector<FileOccurences>>  SymbolOccurences;

As discussed previously, this representation is better suited for both merging and serialization.

clangd/index/SymbolCollector.cpp

272

NIT: maybe use early exits and inverted conditions to keep the nesting down?

321

If we any Options here, why have an extra CollectorSymbolOptions?

clangd/index/SymbolCollector.h

59

Could you elaborate on what this option will be used for? How do we know in advance which symbols we're interested in?

[hokein]

Address review comments.

[hokein]

Add one more comment.

[hokein]

clangd/index/Index.h

46

These operator implementations seem not as much interesting as members in the structure, putting them to the structure probably adds some noise to readers.

348

The file-centric manner doesn't seem to suite our current model: whenever we update the index for the main AST, we just replace the symbol slab with the new one; and for index merging, we only use the index findOccurrences interfaces.

It would save some memory usage of StringRef File, but AFAIK, the memory usage of current model is relatively small (comparing with the SymbolSlab for code completion) since we only store occurrences in main file (~50KB for CodeComplete.cpp).

I'd leave it as it is now, and we could revisit it later.

clangd/index/SymbolCollector.h

59

This is used for finding references in the AST as a part of the xref implementation, basically the workflow would be:

1. find SymbolIDs of the symbol under the cursor, using DeclarationAndMacrosFinder
2. run symbol collector to find all occurrences in the main AST with all SymbolIDs in #1
3. query the index, to get more occurrences
4. merge them

[ilya-biryukov]

clangd/index/Index.h

46

Ok, LG outside too

348

Isn't the merging model different for the occurrences?
We would actually have to drop all references from the older index when merging if the new one contains locations in the same file.
If the merge if file-centric, the file-based representation makes more sense in the first place.

Apart from simpler merging the code, the file-based representation also buys us more efficient serialization for the static index, arguably efficient enough to stash all the occurrences even into our YAML index.

Postponing till later is also fine, but I'm not sure it buys us much now. These arguments only apply if we think the file-centric approach is a the right final design, though.

clangd/index/SymbolCollector.h

59

Can we instead find all the occurences in DeclarationAndMacrosFinder directly?
Extra run of SymbolCollector means another AST traversal, which is slow by itself, and SymbolCollector s designed for a much more hairy problem, its interface is just not nicely suited for things like only occurrences. The latter seems to be a simpler problem, and we can have a simpler interface to solve it (possibly shared between SymbolCollector and DeclarationAndMacrosFinder). WDYT?

[ioeric]

clangd/index/SymbolCollector.h

74

Use llvm::Optional?

[ioeric]

clangd/index/SymbolCollector.cpp

241

I don't see a strong reason for the separation of CollectOccurrence and CollectSymbol. There are some pieceis that are only used by one of them, but they seem cheap enough to ignore? Intuitively, it seems to me reference collection could just be a member function of SymbolCollector.

[sammccall]

clangd/index/Index.h

310

As discussed offline: the merge of occurrences into SymbolSlab seems problematic to me.
On the consumer side, we have a separation between Symbol APIs and SymbolOccurrence APIs - they don't really interact.
The Symbol type can often only be used with SymbolSlab, and so including occurrences drags them into the mess for consumers that don't care about them.

For producers (index implementations), they will usually have both and they may want to share arena storage. But this probably doesn't matter much, and if it does we can use another mechanism (like allowing SymbolSlabBuilder and SymbolOccurrenceSlab to share UniqueStringSaver)

clangd/index/SymbolCollector.cpp

439

note that here we've done basically all the work needed to record the occurrence.

If you add a DenseMap<Decl*, {SourceLocation, SymbolRole}> then you'll have enough info at the end to fill in the occurrences, like we do with referenceddecls -> references.

clangd/index/SymbolCollector.h

40

Not sure this split is justified.
if IDs goes away (see below), all that's left can be represented in a SymbolOccurenceKind filter (which is 0 to collect no occurrences)

59

Yeah, I don't think we need this.
For "find references in the AST" we have an implementation in XRefs for highlights which we don't need to share.

76

collecting symbols doesn't actually need to be optional I think - it's the core responsibility of this class, and "find occurrences of a decl in an ast" can be implemented more easily in other ways

[hokein]

Update the patch based on our new discussion

• SymbolOccurrenceSlab for storing underlying occurrence data
• reuse SymbolCollector to collect symbol occurrences

[sammccall]

This looks pretty good!

clangd/index/Index.h

400

assert frozen?
looking up in a non-frozen array is probably a mistake.
if we choose to optimize this, it probably won't be possible.

401

return Occurrences.lookup(ID)?

clangd/index/SymbolCollector.cpp

228

nit: toOccurrenceKind

230

If you want to filter out the unsupported bits, maybe just add an explicit AllOccurrenceKinds constant to the header file, and return AllOccurrenceKinds & Roles here? (plus casts)

442

just compute the spelling loc once and reuse?

443

you get the spelling loc on the previous line to check for mainfile - so surely we should be using spelling loc here?

564

nit: const auto& for clarity since we're not mutating

569

so this seems maybe gratuitously inefficient, we're copying the filename then going through the URI conversion dance for each reference - even though the filename is the same for each.

consider splitting out part of getTokenLocation into getTokenRange(SymbolLocation&) and only calling that here.

clangd/index/SymbolCollector.h

71–72

this should be next to OccurrenceFilter, they're very closely related (the name mismatch is a little unfortunate)

112

please move next to ReferencedDecls/ReferencedMacros so the comment applies to this too

unittests/clangd/SymbolCollectorTests.cpp

491

this is cute - if possible, consider adding a matcher factory function for readability here, so you can write EXPECT_THAT(..., HaveRanges(Main.ranges("foo"))

[hokein]

Address review comments and fix code style.

[hokein]

Minor cleanup.

[hokein]

clangd/index/Index.h

401

The DenseMap::lookup returns a copy of Value (vector) which doesn't suit our use case :( -- we will return an ArrayRef which stores an reference of a local vector object.

unittests/clangd/SymbolCollectorTests.cpp

491

Wrapped this into HaveRanges.

[hokein]

Address review comments in D51279.

[hokein]

Committed in rL341208.

clangd/index/FileIndex.cpp

 #include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
 
 SymbolSlab indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
                     llvm::ArrayRef<std::string> URISchemes) {
   SymbolCollector::Options CollectorOpts;
+  SymbolCollector::Options::CollectSymbolOptions SymbolOpts;
   // FIXME(ioeric): we might also want to collect include headers. We would need
   // to make sure all includes are canonicalized (with CanonicalIncludes), which
   // is not trivial given the current way of collecting symbols: we only have
   // AST at this point, but we also need preprocessor callbacks (e.g.
   // CommentHandler for IWYU pragma) to canonicalize includes.
-  CollectorOpts.CollectIncludePath = false;
-  CollectorOpts.CountReferences = false;
+  SymbolOpts.CollectIncludePath = false;
+  SymbolOpts.CountReferences = false;
   if (!URISchemes.empty())
     CollectorOpts.URISchemes = URISchemes;
-  CollectorOpts.Origin = SymbolOrigin::Dynamic;
+  SymbolOpts.Origin = SymbolOrigin::Dynamic;
+  CollectorOpts.SymOpts = &SymbolOpts;
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;

clangd/index/Index.h

 
 struct SymbolLocation {
   // Specify a position (Line, Column) of symbol. Using Line/Column allows us to
   // build LSP responses without reading the file content.
   struct Position {
     uint32_t Line = 0; // 0-based
     // Using UTF-16 code units.
     uint32_t Column = 0; // 0-based
-    bool operator==(const Position& P) const {
-      return Line == P.Line && Column == P.Column;
-    }
   };
 
   // The URI of the source file where a symbol occurs.
   llvm::StringRef FileURI;
 
   /// The symbol range, using half-open range [Start, End).
   Position Start;
   Position End;
 
   explicit operator bool() const { return !FileURI.empty(); }
-  bool operator==(const SymbolLocation& Loc) const {
-    return std::tie(FileURI, Start, End) ==
-           std::tie(Loc.FileURI, Loc.Start, Loc.End);
-  }
 };
+inline bool operator==(const SymbolLocation::Position &L,
+                       const SymbolLocation::Position &R) {
+  return std::tie(L.Line, L.Column) == std::tie(R.Line, R.Column);
+}
+inline bool operator<(const SymbolLocation::Position &L,
+                      const SymbolLocation::Position &R) {
+  return std::tie(L.Line, L.Column) < std::tie(R.Line, R.Column);
+}
+inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
+  return std::tie(L.FileURI, L.Start, L.End) ==
+         std::tie(R.FileURI, R.Start, R.End);
+}
+inline bool operator<(const SymbolLocation &L, const SymbolLocation &R) {
+  return std::tie(L.FileURI, L.Start, L.End) <
+         std::tie(R.FileURI, R.Start, R.End);
+}
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
 
 // The class identifies a particular C++ symbol (class, function, method, etc).
 //
 // As USRs (Unified Symbol Resolution) could be large, especially for functions
 // with long type arguments, SymbolID is using 160-bits SHA1(USR) values to
 // guarantee the uniqueness of symbols while using a relatively small amount of
 // memory (vs storing USRs directly).

 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
 
 // Computes query-independent quality score for a Symbol.
 // This currently falls in the range [1, ln(#indexed documents)].
 // FIXME: this should probably be split into symbol -> signals
 //        and signals -> score, so it can be reused for Sema completions.
 double quality(const Symbol &S);
 
+// Describes the kind of a symbol occurrence.
+//
+// This is a bitfield which can be combined from different kinds.
+enum class SymbolOccurrenceKind : 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),
+};
+raw_ostream &operator<<(raw_ostream &OS, SymbolOccurrenceKind K);
+inline SymbolOccurrenceKind operator|(SymbolOccurrenceKind L,
+                                      SymbolOccurrenceKind R) {
+  return static_cast<SymbolOccurrenceKind>(static_cast<uint8_t>(L) |
+                                           static_cast<uint8_t>(R));
+}
+inline SymbolOccurrenceKind &operator|=(SymbolOccurrenceKind &L,
+                                        SymbolOccurrenceKind R) {
+  return L = L | R;
+}
+inline SymbolOccurrenceKind operator&(SymbolOccurrenceKind A,
+                                      SymbolOccurrenceKind B) {
+  return static_cast<SymbolOccurrenceKind>(static_cast<uint8_t>(A) &
+                                           static_cast<uint8_t>(B));
+}
+
+// Represents a symbol occurrence in the source file. It could be a
+// declaration/definition/reference occurrence.
+//
+// WARNING: Location does not own the underlying data - Copies are shallow.
+struct SymbolOccurrence {
+  // The location of the occurrence.
+  SymbolLocation Location;
+  SymbolOccurrenceKind Kind = SymbolOccurrenceKind::Unknown;
+};
+inline bool operator<(const SymbolOccurrence &L, const SymbolOccurrence &R) {
+  return std::tie(L.Location, L.Kind) < std::tie(R.Location, R.Kind);
+}
+inline bool operator==(const SymbolOccurrence &L, const SymbolOccurrence &R) {
+  return std::tie(L.Location, L.Kind) == std::tie(R.Location, R.Kind);
+}
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+                              const SymbolOccurrence &Occurrence);
+
 // An immutable symbol container that stores a set of symbols.
 // The container will maintain the lifetime of the symbols.
 class SymbolSlab {
 public:
   using const_iterator = std::vector<Symbol>::const_iterator;
   using iterator = const_iterator;
 
   SymbolSlab() = default;
 
   const_iterator begin() const { return Symbols.begin(); }
   const_iterator end() const { return Symbols.end(); }
   const_iterator find(const SymbolID &SymID) const;
 
   size_t size() const { return Symbols.size(); }
   // Estimates the total memory usage.
   size_t bytes() const {
     return sizeof(*this) + Arena.getTotalMemory() +
-           Symbols.capacity() * sizeof(Symbol);
+           Symbols.capacity() * sizeof(Symbol) +
+           SymbolOccurrences.getMemorySize();
+  }
+
+  llvm::ArrayRef<SymbolOccurrence> findOccurrences(const SymbolID &ID) const {
+    auto It = SymbolOccurrences.find(ID);
+    if (It == SymbolOccurrences.end())
+      return {};
+    return It->second;
   }
 
   // SymbolSlab::Builder is a mutable container that can 'freeze' to SymbolSlab.
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
     Builder() : UniqueStrings(Arena) {}
 
     // Adds a symbol, overwriting any existing one with the same ID.
     // This is a deep copy: underlying strings will be owned by the slab.
     void insert(const Symbol &S);
 
+    // Adds a symbol occurrence.
+    // This is a deep copy: underlying strings will be owned by the slab.
+    void insert(const SymbolID &ID, SymbolOccurrence Occurrence);
+
     // Returns the symbol with an ID, if it exists. Valid until next insert().
     const Symbol *find(const SymbolID &ID) {
       auto I = SymbolIndex.find(ID);
       return I == SymbolIndex.end() ? nullptr : &Symbols[I->second];
     }
 
     // Consumes the builder to finalize the slab.
     SymbolSlab build() &&;
 
   private:
     llvm::BumpPtrAllocator Arena;
     // Intern table for strings. Contents are on the arena.
     llvm::UniqueStringSaver UniqueStrings;
     std::vector<Symbol> Symbols;
     // Values are indices into Symbols vector.
     llvm::DenseMap<SymbolID, size_t> SymbolIndex;
+    llvm::DenseMap<SymbolID, std::vector<SymbolOccurrence>> SymbolOccurrences;
   };
 
 private:
-  SymbolSlab(llvm::BumpPtrAllocator Arena, std::vector<Symbol> Symbols)
-      : Arena(std::move(Arena)), Symbols(std::move(Symbols)) {}
+  SymbolSlab(
+      llvm::BumpPtrAllocator Arena, std::vector<Symbol> Symbols,
+      llvm::DenseMap<SymbolID, std::vector<SymbolOccurrence>> SymbolOccurrences)
+      : Arena(std::move(Arena)), Symbols(std::move(Symbols)),
+        SymbolOccurrences(std::move(SymbolOccurrences)) {}
 
   llvm::BumpPtrAllocator Arena; // Owns Symbol data that the Symbols do not.
   std::vector<Symbol> Symbols;  // Sorted by SymbolID to allow lookup.
-};
-
-// Describes the kind of a symbol occurrence.
-//
-// This is a bitfield which can be combined from different kinds.
-enum class SymbolOccurrenceKind : 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),
-};
-inline SymbolOccurrenceKind operator|(SymbolOccurrenceKind L,
-                                      SymbolOccurrenceKind R) {
-  return static_cast<SymbolOccurrenceKind>(static_cast<uint8_t>(L) |
-                                           static_cast<uint8_t>(R));
-}
-inline SymbolOccurrenceKind &operator|=(SymbolOccurrenceKind &L,
-                                        SymbolOccurrenceKind R) {
-  return L = L | R;
-}
-inline SymbolOccurrenceKind operator&(SymbolOccurrenceKind A,
-                                      SymbolOccurrenceKind B) {
-  return static_cast<SymbolOccurrenceKind>(static_cast<uint8_t>(A) &
-                                           static_cast<uint8_t>(B));
-}
-
-// Represents a symbol occurrence in the source file. It could be a
-// declaration/definition/reference occurrence.
-//
-// WARNING: Location does not own the underlying data - Copies are shallow.
-struct SymbolOccurrence {
-  // The location of the occurrence.
-  SymbolLocation Location;
-  SymbolOccurrenceKind Kind = SymbolOccurrenceKind::Unknown;
+  llvm::DenseMap<SymbolID, std::vector<SymbolOccurrence>> SymbolOccurrences;
 };
 
 struct FuzzyFindRequest {
   /// \brief A query string for the fuzzy find. This is matched against symbols'
   /// un-qualified identifiers and should not contain qualifiers like "::".
   std::string Query;
   /// \brief If this is non-empty, symbols must be in at least one of the scopes
   /// (e.g. namespaces) excluding nested scopes. For example, if a scope "xyz::"

 /// \brief Interface for symbol indexes that can be used for searching or
 /// matching symbols among a set of symbols based on names or unique IDs.
 class SymbolIndex {
 public:
   virtual ~SymbolIndex() = default;
 
   /// \brief Matches symbols in the index fuzzily and applies \p Callback on
   /// each matched symbol before returning.
   /// If returned Symbols are used outside Callback, they must be deep-copied!
   ///
   /// Returns true if there may be more results (limited by MaxCandidateCount).
   virtual bool
   fuzzyFind(const FuzzyFindRequest &Req,
             llvm::function_ref<void(const Symbol &)> Callback) const = 0;
 
   /// Looks up symbols with any of the given symbol IDs and applies \p Callback
   /// on each matched symbol.
   /// The returned symbol must be deep-copied if it's used outside Callback.

clangd/index/Index.cpp

void SymbolSlab::Builder::insert(const Symbol &S) {
   if (R.second) {
     Symbols.push_back(S);
     own(Symbols.back(), UniqueStrings, Arena);
   } else {
     auto &Copy = Symbols[R.first->second] = S;
     own(Copy, UniqueStrings, Arena);
   }
 }
+void SymbolSlab::Builder::insert(const SymbolID &ID,
+                                 SymbolOccurrence Occurrence) {
+  Occurrence.Location.FileURI = UniqueStrings.save(Occurrence.Location.FileURI);
+  SymbolOccurrences[ID].push_back(std::move(Occurrence));
+}
 
 SymbolSlab SymbolSlab::Builder::build() && {
   Symbols = {Symbols.begin(), Symbols.end()}; // Force shrink-to-fit.
   // Sort symbols so the slab can binary search over them.
   std::sort(Symbols.begin(), Symbols.end(),
             [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   BumpPtrAllocator NewArena;
   llvm::UniqueStringSaver Strings(NewArena);
   for (auto &S : Symbols)
     own(S, Strings, NewArena);
-  return SymbolSlab(std::move(NewArena), std::move(Symbols));
+
+  // We may have duplicated symbol occurrences (as some AST nodes have been
+  // visited multiple times). Deduplicate them.
+  for (auto &IDAndOccurrences : SymbolOccurrences) {
+    auto &Occurrences = IDAndOccurrences.getSecond();
+    std::sort(Occurrences.begin(), Occurrences.end());
+    Occurrences.erase(std::unique(Occurrences.begin(), Occurrences.end()),
+                      Occurrences.end());
+
+    for (auto &O : Occurrences)
+      O.Location.FileURI = UniqueStrings.save(O.Location.FileURI);
+  }
+
+  return SymbolSlab(std::move(NewArena), std::move(Symbols),
+                    std::move(SymbolOccurrences));
+}
+
+raw_ostream &operator<<(raw_ostream &OS, SymbolOccurrenceKind K) {
+  if (K == SymbolOccurrenceKind::Unknown)
+    return OS << "Unknown";
+  static const std::vector<const char*> Messages = {
+    "Declaration",
+    "Definition",
+    "Reference"
+  };
+  bool VisitedOnce = false;
+  for (unsigned I = 0; I < Messages.size(); ++I) {
+    if (static_cast<uint8_t>(K) & 1u << I) {
+      if (VisitedOnce)
+        OS << ", ";
+      OS << Messages[I];
+      VisitedOnce = true;
+    }
+  }
+  return OS;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+                              const SymbolOccurrence &Occurrence) {
+  OS << Occurrence.Location << ":" << Occurrence.Kind;
+  return OS;
 }
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.h

 /// See also shouldCollectSymbol(...).
 ///
 /// Clients (e.g. clangd) can use SymbolCollector together with
 /// index::indexTopLevelDecls to retrieve all symbols when the source file is
 /// changed.
 class SymbolCollector : public index::IndexDataConsumer {
 public:
   struct Options {
-    /// When symbol paths cannot be resolved to absolute paths (e.g. files in
-    /// VFS that does not have absolute path), combine the fallback directory
-    /// with symbols' paths to get absolute paths. This must be an absolute
-    /// path.
-    std::string FallbackDir;
-    /// Specifies URI schemes that can be used to generate URIs for file paths
-    /// in symbols. The list of schemes will be tried in order until a working
-    /// scheme is found. If no scheme works, symbol location will be dropped.
-    std::vector<std::string> URISchemes = {"file"};
+    struct CollectSymbolOptions {
       bool CollectIncludePath = false;
       /// If set, this is used to map symbol #include path to a potentially
       /// different #include path.
       const CanonicalIncludes *Includes = nullptr;
       // Populate the Symbol.References field.
       bool CountReferences = 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;
     };
+    struct CollectOccurrenceOptions {
+      SymbolOccurrenceKind Filter;
+      // A whitelist symbols which will be collected.
+      // If none, all symbol occurrences will be collected.
+      llvm::Optional<llvm::DenseSet<SymbolID>> IDs = llvm::None;
+    };
+
+    /// Specifies URI schemes that can be used to generate URIs for file paths
+    /// in symbols. The list of schemes will be tried in order until a working
+    /// scheme is found. If no scheme works, symbol location will be dropped.
+    std::vector<std::string> URISchemes = {"file"};
+
+    /// When symbol paths cannot be resolved to absolute paths (e.g. files in
+    /// VFS that does not have absolute path), combine the fallback directory
+    /// with symbols' paths to get absolute paths. This must be an absolute
+    /// path.
+    std::string FallbackDir;
+
+    // If not null, SymbolCollector will collect symbols.
+    const CollectSymbolOptions *SymOpts;
+    // If not null, SymbolCollector will collect symbol occurrences.
+    const CollectOccurrenceOptions *OccurrenceOpts;
+  };
 
   SymbolCollector(Options Opts);
 
+  ~SymbolCollector();
+
   /// Returns true is \p ND should be collected.
   /// AST matchers require non-const ASTContext.
-  static bool shouldCollectSymbol(const NamedDecl &ND, ASTContext &ASTCtx,
-                                  const Options &Opts);
+  static bool shouldCollectSymbol(const NamedDecl &ND, ASTContext &ASTCtx);
 
   void initialize(ASTContext &Ctx) override;
 
-  void setPreprocessor(std::shared_ptr<Preprocessor> PP) override {
-    this->PP = std::move(PP);
-  }
+  void setPreprocessor(std::shared_ptr<Preprocessor> PP) override;
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                       ArrayRef<index::SymbolRelation> Relations,
                       SourceLocation Loc,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   bool handleMacroOccurence(const IdentifierInfo *Name, const MacroInfo *MI,
                             index::SymbolRoleSet Roles,
                             SourceLocation Loc) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
   void finish() override;
 
 private:
-  const Symbol *addDeclaration(const NamedDecl &, SymbolID);
-  void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
+  Options Opts;
 
-  // All Symbols collected from the AST.
-  SymbolSlab::Builder Symbols;
-  ASTContext *ASTCtx;
   std::shared_ptr<Preprocessor> PP;
-  std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
-  std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
-  Options Opts;
-  // Symbols referenced from the current TU, flushed on finish().
-  llvm::DenseSet<const NamedDecl *> ReferencedDecls;
-  llvm::DenseSet<const IdentifierInfo *> ReferencedMacros;
-  // 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;
+
+  class CollectSymbol;
+  class CollectOccurrence;
+  std::unique_ptr<CollectSymbol> CollectSym;
+  std::unique_ptr<CollectOccurrence> CollectOccur;
+  // All symbols and symbol occurrences collected from the AST.
+  SymbolSlab::Builder Symbols;
 };
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.cpp

while (true) {
     Headers.push_back(FilePath);
     if (SM.isInMainFile(Loc))
       break;
     Loc = SM.getIncludeLoc(SM.getFileID(Loc));
   }
   if (Headers.empty())
     return llvm::None;
   llvm::StringRef Header = Headers[0];
-  if (Opts.Includes) {
-    Header = Opts.Includes->mapHeader(Headers, QName);
+  assert(Opts.SymOpts && "SymbolOptions must be set.");
+  if (Opts.SymOpts->Includes) {
+    Header = Opts.SymOpts->Includes->mapHeader(Headers, QName);
     if (Header.startswith("<") || Header.startswith("\""))
       return Header.str();
   }
   return toURI(SM, Header, Opts);
 }
 
 // Return the symbol location of the token at \p Loc.
 llvm::Optional<SymbolLocation>

 // heuristic.
 bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) {
   using namespace clang::ast_matchers;
   return (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) &&
          llvm::isa<TagDecl>(&ND) &&
          match(decl(isExpansionInMainFile()), ND, ND.getASTContext()).empty();
 }
 
+SymbolOccurrenceKind ToOccurrenceKind(index::SymbolRoleSet Roles) {
+  SymbolOccurrenceKind Kind;
+  for (auto Mask :
+       {SymbolOccurrenceKind::Declaration, SymbolOccurrenceKind::Definition,
+        SymbolOccurrenceKind::Reference}) {
+    if (Roles & static_cast<unsigned>(Mask))
+      Kind |= Mask;
+  }
+  return Kind;
+}
+
 } // namespace
 
-SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {}
+class SymbolCollector::CollectOccurrence {
+public:
+  CollectOccurrence(const SymbolCollector::Options &CollectorOpts,
+                    SymbolSlab::Builder *Builder)
+      : Opts(CollectorOpts), Builder(Builder) {
+    assert(Opts.OccurrenceOpts && "Occurrence options must be set.");
+  }
 
-void SymbolCollector::initialize(ASTContext &Ctx) {
+  void initialize(ASTContext &Ctx) { ASTCtx = &Ctx; }
+
+  void collectDecl(const Decl *D, index::SymbolRoleSet Roles,
+                   ArrayRef<index::SymbolRelation> Relations,
+                   SourceLocation Loc,
+                   index::IndexDataConsumer::ASTNodeInfo ASTNode) {
+    assert(ASTCtx && "ASTContext must be set.");
+    if (D->isImplicit())
+      return;
+
+    // We only collect symbol occurrences in current main file.
+    if (!ASTCtx->getSourceManager().isInMainFile(Loc))
+      return;
+    std::string FileURI;
+    auto AddOccurrence = [&](SourceLocation L, const SymbolID &ID) {
+      if (auto Location =
+              getTokenLocation(Loc, ASTCtx->getSourceManager(), Opts,
+                               ASTCtx->getLangOpts(), FileURI)) {
+        SymbolOccurrence Occurrence;
+        Occurrence.Location = *Location;
+        Occurrence.Kind = ToOccurrenceKind(Roles);
+        Builder->insert(ID, Occurrence);
+      }
+    };
+    if (!(static_cast<unsigned>(Opts.OccurrenceOpts->Filter) & Roles))
+      return;
+
+    if (auto ID = getSymbolID(D)) {
+      if (!Opts.OccurrenceOpts->IDs ||
+          llvm::is_contained(*Opts.OccurrenceOpts->IDs, *ID))
+        AddOccurrence(Loc, *ID);
+    }
+  }
+
+private:
+  const SymbolCollector::Options &Opts;
+
+  SymbolSlab::Builder *Builder;
+  ASTContext *ASTCtx;
+};
+
+class SymbolCollector::CollectSymbol {
+public:
+  CollectSymbol(const SymbolCollector::Options &CollectorOpts,
+                SymbolSlab::Builder *Builder)
+      : Opts(CollectorOpts), Symbols(Builder) {
+    assert(Opts.SymOpts && "Symbol option must be set.");
+  }
+
+  void collectDecl(const Decl *D, index::SymbolRoleSet Roles,
+                   ArrayRef<index::SymbolRelation> Relations,
+                   SourceLocation Loc,
+                   index::IndexDataConsumer::ASTNodeInfo ASTNode);
+
+  void collectMacro(const IdentifierInfo *Name, const MacroInfo *MI,
+                    index::SymbolRoleSet Roles, SourceLocation Loc);
+
+  void initialize(ASTContext &Ctx) {
     ASTCtx = &Ctx;
     CompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>();
     CompletionTUInfo =
         llvm::make_unique<CodeCompletionTUInfo>(CompletionAllocator);
   }
 
+  void setPreprocessor(std::shared_ptr<Preprocessor> PP) {
+    this->PP = std::move(PP);
+  }
+
+  void finish();
+
+private:
+  const Symbol *addDeclaration(const NamedDecl &, SymbolID);
+  void addDefinition(const NamedDecl &ND, const Symbol &DeclSym);
+
+  const SymbolCollector::Options &Opts;
+
+  SymbolSlab::Builder *Symbols;
+  ASTContext *ASTCtx;
+
+  std::shared_ptr<Preprocessor> PP;
+  std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
+  std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
+  // Symbols referenced from the current TU, flushed on finish().
+  llvm::DenseSet<const NamedDecl *> ReferencedDecls;
+  llvm::DenseSet<const IdentifierInfo *> ReferencedMacros;
+  // 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;
+};
+
+SymbolCollector::SymbolCollector(Options CollectorOpts)
+    : Opts(std::move(CollectorOpts)) {
+  if (this->Opts.SymOpts)
+    CollectSym = llvm::make_unique<CollectSymbol>(Opts, &Symbols);
+  if (this->Opts.OccurrenceOpts)
+    CollectOccur = llvm::make_unique<CollectOccurrence>(Opts, &Symbols);
+}
+
+SymbolCollector::~SymbolCollector() {}
+
+void SymbolCollector::initialize(ASTContext &Ctx) {
+  if (CollectSym)
+    CollectSym->initialize(Ctx);
+  if (CollectOccur)
+    CollectOccur->initialize(Ctx);
+}
+
+void SymbolCollector::setPreprocessor(std::shared_ptr<Preprocessor> PP) {
+  if (CollectSym)
+    CollectSym->setPreprocessor(PP);
+}
+
 bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
-                                          ASTContext &ASTCtx,
-                                          const Options &Opts) {
+                                          ASTContext &ASTCtx) {
   using namespace clang::ast_matchers;
   if (ND.isImplicit())
     return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).
   if (ND.getDeclName().isEmpty())
     return false;
 
   // FIXME: figure out a way to handle internal linkage symbols (e.g. static
bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
 
   // Avoid indexing internal symbols in protobuf generated headers.
   if (isPrivateProtoDecl(ND))
     return false;
   return true;
 }
 
 // Always return true to continue indexing.
-bool SymbolCollector::handleDeclOccurence(
+void SymbolCollector::CollectSymbol::collectDecl(
     const Decl *D, index::SymbolRoleSet Roles,
     ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
   assert(ASTNode.OrigD);
   // If OrigD is an declaration associated with a friend declaration and it's
   // not a definition, skip it. Note that OrigD is the occurrence that the
   // collector is currently visiting.
   if ((ASTNode.OrigD->getFriendObjectKind() !=
        Decl::FriendObjectKind::FOK_None) &&
       !(Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
-    return true;
+    return;
   // A declaration created for a friend declaration should not be used as the
   // canonical declaration in the index. Use OrigD instead, unless we've already
   // picked a replacement for D
   if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
     D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second;
   const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D);
   if (!ND)
-    return true;
+    return;
 
   // 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 &&
+  if (Opts.SymOpts->CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
       SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
 
   // Don't continue indexing if this is a mere reference.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
-    return true;
-  if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
-    return true;
+    return;
+  if (!shouldCollectSymbol(*ND, *ASTCtx))
+    return;
 
   auto ID = getSymbolID(ND);
   if (!ID)
-    return true;
+    return;
 
   const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD);
-  const Symbol *BasicSymbol = Symbols.find(*ID);
+  const Symbol *BasicSymbol = Symbols->find(*ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
     BasicSymbol = addDeclaration(*ND, std::move(*ID));
   else if (isPreferredDeclaration(OriginalDecl, Roles))
     // If OriginalDecl is preferred, replace the existing canonical
     // declaration (e.g. a class forward declaration). There should be at most
     // one duplicate as we expect to see only one preferred declaration per
     // TU, because in practice they are definitions.
     BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID));
 
   if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
     addDefinition(OriginalDecl, *BasicSymbol);
-  return true;
 }
 
-bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
+void SymbolCollector::CollectSymbol::collectMacro(const IdentifierInfo *Name,
                                                   const MacroInfo *MI,
                                                   index::SymbolRoleSet Roles,
                                                   SourceLocation Loc) {
-  if (!Opts.CollectMacro)
-    return true;
+  if (!Opts.SymOpts->CollectMacro)
+    return;
   assert(PP.get());
 
   const auto &SM = PP->getSourceManager();
   if (SM.isInMainFile(SM.getExpansionLoc(MI->getDefinitionLoc())))
-    return true;
+    return;
   // Header guards are not interesting in index. Builtin macros don't have
   // useful locations and are not needed for code completions.
   if (MI->isUsedForHeaderGuard() || MI->isBuiltinMacro())
-    return true;
+    return;
 
   // 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 &&
+  if (Opts.SymOpts->CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
       SM.getFileID(SM.getSpellingLoc(Loc)) == 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;
+    return;
 
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM,
                                  USR))
-    return true;
+    return;
   SymbolID ID(USR);
 
   // Only collect one instance in case there are multiple.
-  if (Symbols.find(ID) != nullptr)
-    return true;
+  if (Symbols->find(ID) != nullptr)
+    return;
 
   Symbol S;
   S.ID = std::move(ID);
   S.Name = Name->getName();
   S.IsIndexedForCodeCompletion = true;
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   std::string FileURI;
   if (auto DeclLoc = getTokenLocation(MI->getDefinitionLoc(), SM, Opts,
                                       PP->getLangOpts(), FileURI))
     S.CanonicalDeclaration = *DeclLoc;
 
   CodeCompletionResult SymbolCompletion(Name);
   const auto *CCS = SymbolCompletion.CreateCodeCompletionStringForMacro(
       *PP, *CompletionAllocator, *CompletionTUInfo);
   std::string Signature;
   std::string SnippetSuffix;
   getSignature(*CCS, &Signature, &SnippetSuffix);
 
   std::string Include;
-  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
+  if (Opts.SymOpts->CollectIncludePath &&
+      shouldCollectIncludePath(S.SymInfo.Kind)) {
     if (auto Header =
             getIncludeHeader(Name->getName(), SM,
                              SM.getExpansionLoc(MI->getDefinitionLoc()), Opts))
       Include = std::move(*Header);
   }
   S.Signature = Signature;
   S.CompletionSnippetSuffix = SnippetSuffix;
   Symbol::Details Detail;
   Detail.IncludeHeader = Include;
   S.Detail = &Detail;
-  Symbols.insert(S);
-  return true;
+  Symbols->insert(S);
 }
 
-void SymbolCollector::finish() {
+void SymbolCollector::CollectSymbol::finish() {
   // At the end of the TU, add 1 to the refcount of all referenced symbols.
   auto IncRef = [this](const SymbolID &ID) {
-    if (const auto *S = Symbols.find(ID)) {
+    if (const auto *S = Symbols->find(ID)) {
       Symbol Inc = *S;
       ++Inc.References;
-      Symbols.insert(Inc);
+      Symbols->insert(Inc);
     }
   };
   for (const NamedDecl *ND : ReferencedDecls) {
     if (auto ID = getSymbolID(ND)) {
       IncRef(*ID);
     }
   }
-  if (Opts.CollectMacro) {
+  if (Opts.SymOpts->CollectMacro) {
     assert(PP);
     for (const IdentifierInfo *II : ReferencedMacros) {
       llvm::SmallString<128> USR;
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (!index::generateUSRForMacro(II->getName(), MI->getDefinitionLoc(),
                                         PP->getSourceManager(), USR))
           IncRef(SymbolID(USR));
     }
   }
   ReferencedDecls.clear();
   ReferencedMacros.clear();
 }
 
-const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
+const Symbol *
+SymbolCollector::CollectSymbol::addDeclaration(const NamedDecl &ND,
                                                SymbolID ID) {
   auto &Ctx = ND.getASTContext();
   auto &SM = Ctx.getSourceManager();
 
   Symbol S;
   S.ID = std::move(ID);
   std::string QName = printQualifiedName(ND);
   std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
   // FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
SymbolCollector::CollectSymbol::addDeclaration(const NamedDecl &ND,
   std::string SnippetSuffix;
   getSignature(*CCS, &Signature, &SnippetSuffix);
   std::string Documentation =
       formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
                                               /*CommentsFromHeaders=*/true));
   std::string ReturnType = getReturnType(*CCS);
 
   std::string Include;
-  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
+  if (Opts.SymOpts->CollectIncludePath &&
+      shouldCollectIncludePath(S.SymInfo.Kind)) {
     // Use the expansion location to get the #include header since this is
     // where the symbol is exposed.
     if (auto Header = getIncludeHeader(
             QName, SM, SM.getExpansionLoc(ND.getLocation()), Opts))
       Include = std::move(*Header);
   }
   S.Signature = Signature;
   S.CompletionSnippetSuffix = SnippetSuffix;
   Symbol::Details Detail;
   Detail.Documentation = Documentation;
   Detail.ReturnType = ReturnType;
   Detail.IncludeHeader = Include;
   S.Detail = &Detail;
 
-  S.Origin = Opts.Origin;
-  Symbols.insert(S);
-  return Symbols.find(S.ID);
+  S.Origin = Opts.SymOpts->Origin;
+  Symbols->insert(S);
+  return Symbols->find(S.ID);
 }
 
-void SymbolCollector::addDefinition(const NamedDecl &ND,
+void SymbolCollector::CollectSymbol::addDefinition(const NamedDecl &ND,
                                                    const Symbol &DeclSym) {
   if (DeclSym.Definition)
     return;
   // If we saw some forward declaration, we end up copying the symbol.
   // This is not ideal, but avoids duplicating the "is this a definition" check
   // in clang::index. We should only see one definition.
   Symbol S = DeclSym;
   std::string FileURI;
   if (auto DefLoc = getTokenLocation(findNameLoc(&ND),
                                      ND.getASTContext().getSourceManager(),
                                      Opts, ASTCtx->getLangOpts(), FileURI))
     S.Definition = *DefLoc;
-  Symbols.insert(S);
+  Symbols->insert(S);
+}
+
+bool SymbolCollector::handleDeclOccurence(
+    const Decl *D, index::SymbolRoleSet Roles,
+    ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
+    index::IndexDataConsumer::ASTNodeInfo ASTNode) {
+  if (CollectSym)
+    CollectSym->collectDecl(D, Roles, Relations, Loc, ASTNode);
+  if (CollectOccur)
+    CollectOccur->collectDecl(D, Roles, Relations, Loc, ASTNode);
+  return true;
+}
+
+bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
+                                           const MacroInfo *MI,
+                                           index::SymbolRoleSet Roles,
+                                           SourceLocation Loc) {
+  if (CollectSym)
+    CollectSym->collectMacro(Name, MI, Roles, Loc);
+  return true;
+}
+
+void SymbolCollector::finish() {
+  if (CollectSym)
+    CollectSym->finish();
 }
 
 } // namespace clangd
 } // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

return std::tie(arg.Definition.Start.Line,
                   arg.Definition.End.Column) ==
          std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
                   Pos.end.character);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
 }
+MATCHER(OccurrenceRange, "") {
+  const clang::clangd::SymbolOccurrence &Pos = testing::get<0>(arg);
+  const clang::clangd::Range &Range = testing::get<1>(arg);
+  return std::tie(Pos.Location.Start.Line, Pos.Location.Start.Column,
+                  Pos.Location.End.Line, Pos.Location.End.Column) ==
+         std::tie(Range.start.line, Range.start.character, Range.end.line,
+                  Range.end.character);
+}
 
 namespace clang {
 namespace clangd {
 
 namespace {
 
 class ShouldCollectSymbolTest : public ::testing::Test {
 public:
   void build(StringRef HeaderCode, StringRef Code = "") {
     File.HeaderFilename = HeaderName;
     File.Filename = FileName;
     File.HeaderCode = HeaderCode;
     File.Code = Code;
     AST = File.build();
   }
 
   // build() must have been called.
   bool shouldCollect(StringRef Name, bool Qualified = true) {
     assert(AST.hasValue());
     return SymbolCollector::shouldCollectSymbol(
         Qualified ? findDecl(*AST, Name) : findAnyDecl(*AST, Name),
-        AST->getASTContext(), SymbolCollector::Options());
+        AST->getASTContext());
   }
 
 protected:
   std::string HeaderName = "f.h";
   std::string FileName = "f.cpp";
   TestTU File;
   Optional<ParsedAST> AST;  // Initialized after build.
 };
TEST_F(ShouldCollectSymbolTest, DoubleCheckProtoHeaderComment) {
   )");
   EXPECT_TRUE(shouldCollect("nx::Top_Level"));
   EXPECT_TRUE(shouldCollect("nx::Kind_Fine"));
 }
 
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
   SymbolIndexActionFactory(SymbolCollector::Options COpts,
-                           CommentHandler *PragmaHandler)
-      : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
+                           CommentHandler *PragmaHandler,
+                           index::IndexingOptions IndexOpts)
+      : COpts(std::move(COpts)), PragmaHandler(PragmaHandler),
+        IndexOpts(IndexOpts) {}
 
   clang::FrontendAction *create() override {
     class WrappedIndexAction : public WrapperFrontendAction {
     public:
       WrappedIndexAction(std::shared_ptr<SymbolCollector> C,
                          const index::IndexingOptions &Opts,
                          CommentHandler *PragmaHandler)
           : WrapperFrontendAction(
                 index::createIndexingAction(C, Opts, nullptr)),
             PragmaHandler(PragmaHandler) {}
 
       std::unique_ptr<ASTConsumer>
       CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
         if (PragmaHandler)
           CI.getPreprocessor().addCommentHandler(PragmaHandler);
         return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
       }
 
     private:
       index::IndexingOptions IndexOpts;
       CommentHandler *PragmaHandler;
     };
-    index::IndexingOptions IndexOpts;
-    IndexOpts.SystemSymbolFilter =
-        index::IndexingOptions::SystemSymbolFilterKind::All;
-    IndexOpts.IndexFunctionLocals = false;
     Collector = std::make_shared<SymbolCollector>(COpts);
     return new WrappedIndexAction(Collector, std::move(IndexOpts),
                                   PragmaHandler);
   }
 
   std::shared_ptr<SymbolCollector> Collector;
   SymbolCollector::Options COpts;
   CommentHandler *PragmaHandler;
+
+  index::IndexingOptions IndexOpts;
 };
 
 class SymbolCollectorTest : public ::testing::Test {
 public:
   SymbolCollectorTest()
       : InMemoryFileSystem(new vfs::InMemoryFileSystem),
         TestHeaderName(testPath("symbol.h")),
         TestFileName(testPath("symbol.cc")) {
     TestHeaderURI = URI::createFile(TestHeaderName).toString();
     TestFileURI = URI::createFile(TestFileName).toString();
   }
 
+  bool collectSymbols(StringRef HeaderCode, StringRef MainCode,
+                      const std::vector<std::string> &ExtraArgs = {}) {
+    index::IndexingOptions IndexOpts;
+    IndexOpts.SystemSymbolFilter =
+        index::IndexingOptions::SystemSymbolFilterKind::All;
+    IndexOpts.IndexFunctionLocals = false;
+    CollectorOpts.SymOpts = &CollectSymOpts;
+    CollectorOpts.OccurrenceOpts = nullptr;
+    return runSymbolCollector(HeaderCode, MainCode, CollectorOpts, IndexOpts,
+                              ExtraArgs);
+  }
+
+  bool collectOccurrences(StringRef HeaderCode, StringRef MainCode,
+                          const std::vector<std::string> &ExtraArgs = {}) {
+    index::IndexingOptions IndexOpts;
+    IndexOpts.SystemSymbolFilter =
+        index::IndexingOptions::SystemSymbolFilterKind::All;
+    IndexOpts.IndexFunctionLocals = true;
+    CollectorOpts.SymOpts = nullptr;
+    CollectorOpts.OccurrenceOpts = &CollectOccurrenceOpts;
+    return runSymbolCollector(HeaderCode, MainCode, CollectorOpts, IndexOpts,
+                              ExtraArgs);
+  }
+
+protected:
+  llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem;
+  std::string TestHeaderName;
+  std::string TestHeaderURI;
+  std::string TestFileName;
+  std::string TestFileURI;
+  SymbolSlab Symbols;
+  SymbolCollector::Options CollectorOpts;
+  SymbolCollector::Options::CollectSymbolOptions CollectSymOpts;
+  SymbolCollector::Options::CollectOccurrenceOptions CollectOccurrenceOpts;
+  std::unique_ptr<CommentHandler> PragmaHandler;
+
+private:
   bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode,
+                          SymbolCollector::Options Opts,
+                          index::IndexingOptions IndexOpts,
                           const std::vector<std::string> &ExtraArgs = {}) {
     llvm::IntrusiveRefCntPtr<FileManager> Files(
         new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
     auto Factory = llvm::make_unique<SymbolIndexActionFactory>(
-        CollectorOpts, PragmaHandler.get());
+        Opts, PragmaHandler.get(), std::move(IndexOpts));
 
     std::vector<std::string> Args = {
         "symbol_collector", "-fsyntax-only", "-xc++",
         "-std=c++11",       "-include",      TestHeaderName};
     Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
     // This allows to override the "-xc++" with something else, i.e.
     // -xobjective-c++.
     Args.push_back(TestFileName);
 
     tooling::ToolInvocation Invocation(
         Args,
         Factory->create(), Files.get(),
         std::make_shared<PCHContainerOperations>());
 
     InMemoryFileSystem->addFile(TestHeaderName, 0,
                                 llvm::MemoryBuffer::getMemBuffer(HeaderCode));
     InMemoryFileSystem->addFile(TestFileName, 0,
                                 llvm::MemoryBuffer::getMemBuffer(MainCode));
     Invocation.run();
     Symbols = Factory->Collector->takeSymbols();
     return true;
   }
-
-protected:
-  llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem;
-  std::string TestHeaderName;
-  std::string TestHeaderURI;
-  std::string TestFileName;
-  std::string TestFileURI;
-  SymbolSlab Symbols;
-  SymbolCollector::Options CollectorOpts;
-  std::unique_ptr<CommentHandler> PragmaHandler;
 };
 
 TEST_F(SymbolCollectorTest, CollectSymbols) {
   const std::string Header = R"(
     class Foo {
       Foo() {}
       Foo(int a) {}
       void f();
const std::string Header = R"(
     namespace baz = bar;
 
     // FIXME: using declaration is not supported as the IndexAction will ignore
     // implicit declarations (the implicit using shadow declaration) by default,
     // and there is no way to customize this behavior at the moment.
     using bar::v2;
     } // namespace foo
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAreArray(
                   {AllOf(QName("Foo"), ForCodeCompletion(true)),
                    AllOf(QName("Foo::Foo"), ForCodeCompletion(false)),
                    AllOf(QName("Foo::Foo"), ForCodeCompletion(false)),
                    AllOf(QName("Foo::f"), ForCodeCompletion(false)),
                    AllOf(QName("Foo::~Foo"), ForCodeCompletion(false)),
                    AllOf(QName("Foo::operator="), ForCodeCompletion(false)),

 TEST_F(SymbolCollectorTest, Template) {
   Annotations Header(R"(
     // Template is indexed, specialization and instantiation is not.
     template <class T> struct [[Tmpl]] {T $xdecl[[x]] = 0;};
     template <> struct Tmpl<int> {};
     extern template struct Tmpl<float>;
     template struct Tmpl<double>;
   )");
-  runSymbolCollector(Header.code(), /*Main=*/"");
+  collectSymbols(Header.code(), /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAreArray(
                   {AllOf(QName("Tmpl"), DeclRange(Header.range())),
                    AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")))}));
 }
 
 TEST_F(SymbolCollectorTest, ObjCSymbols) {
   const std::string Header = R"(
const std::string Header = R"(
     }
     @end
 
     @protocol MyProtocol
     - (void)someMethodName3:(void*)name3;
     @end
   )";
   TestFileName = "test.m";
-  runSymbolCollector(Header, /*Main=*/"", {"-fblocks", "-xobjective-c++"});
+  collectSymbols(Header, /*Main=*/"", {"-fblocks", "-xobjective-c++"});
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   QName("Person"), QName("Person::someMethodName:lastName:"),
                   QName("MyCategory"), QName("Person::someMethodName2:"),
                   QName("MyProtocol"), QName("MyProtocol::someMethodName3:")));
 }
 
 TEST_F(SymbolCollectorTest, Locations) {
o]]();
   Annotations Main(R"cpp(
     int $xdef[[X]] = 42;
     class $clsdef[[Cls]] {};
     void $printdef[[print]]() {}
 
     // Declared/defined in main only.
     int Y;
   )cpp");
-  runSymbolCollector(Header.code(), Main.code());
+  collectSymbols(Header.code(), Main.code());
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
           AllOf(QName("X"), DeclRange(Header.range("xdecl")),
                 DefRange(Main.range("xdef"))),
           AllOf(QName("Cls"), DeclRange(Header.range("clsdecl")),
                 DefRange(Main.range("clsdef"))),
           AllOf(QName("print"), DeclRange(Header.range("printdecl")),
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);
+
+  CollectSymOpts.CountReferences = true;
+  collectSymbols(Header, Main);
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(QName("W"), Refs(1)),
                                    AllOf(QName("X"), Refs(1)),
                                    AllOf(QName("Y"), Refs(0)),
                                    AllOf(QName("Z"), Refs(0)), QName("y")));
 }
 
 TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("Foo"), DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
   TestHeaderName = "x.h";
   TestFileName = "x.cpp";
   TestHeaderURI = URI::createFile(testPath(TestHeaderName)).toString();
   CollectorOpts.FallbackDir = testRoot();
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, CustomURIScheme) {
   // Use test URI scheme from URITests.cpp
   CollectorOpts.URISchemes.insert(CollectorOpts.URISchemes.begin(), "unittest");
   TestHeaderName = testPath("x.h");
   TestFileName = testPath("x.cpp");
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("Foo"), DeclURI("unittest:///x.h"))));
 }
 
 TEST_F(SymbolCollectorTest, InvalidURIScheme) {
   // Use test URI scheme from URITests.cpp
   CollectorOpts.URISchemes = {"invalid"};
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(""))));
 }
 
 TEST_F(SymbolCollectorTest, FallbackToFileURI) {
   // Use test URI scheme from URITests.cpp
   CollectorOpts.URISchemes = {"invalid", "file"};
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("Foo"), DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
   const std::string Header = R"(
     enum {
       Red
     };
     enum Color {
       Green
     };
     enum class Color2 {
       Yellow
     };
     namespace ns {
     enum {
       Black
     };
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   AllOf(QName("Red"), ForCodeCompletion(true)),
                   AllOf(QName("Color"), ForCodeCompletion(true)),
                   AllOf(QName("Green"), ForCodeCompletion(true)),
                   AllOf(QName("Color2"), ForCodeCompletion(true)),
                   AllOf(QName("Color2::Yellow"), ForCodeCompletion(false)),
                   AllOf(QName("ns"), ForCodeCompletion(true)),
                   AllOf(QName("ns::Black"), ForCodeCompletion(true))));
 }
 
 TEST_F(SymbolCollectorTest, NamelessSymbols) {
   const std::string Header = R"(
     struct {
       int a;
     } Foo;
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"),
                                             QName("(anonymous struct)::a")));
 }
 
 TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) {
 
   Annotations Header(R"(
     #define FF(name) \
       class name##_Test {};
 
     $expansion[[FF]](abc);
 
     #define FF2() \
       class $spelling[[Test]] {};
 
     FF2();
   )");
 
-  runSymbolCollector(Header.code(), /*Main=*/"");
+  collectSymbols(Header.code(), /*Main=*/"");
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
           AllOf(QName("abc_Test"), DeclRange(Header.range("expansion")),
                 DeclURI(TestHeaderURI)),
           AllOf(QName("Test"), DeclRange(Header.range("spelling")),
                 DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
   Annotations Header(R"(
     #ifdef NAME
     class $expansion[[NAME]] {};
     #endif
   )");
 
-  runSymbolCollector(Header.code(), /*Main=*/"",
-                     /*ExtraArgs=*/{"-DNAME=name"});
+  collectSymbols(Header.code(), /*Main=*/"",
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(
                   QName("name"),
                   DeclRange(Header.range("expansion")),
                   DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
   const std::string Header = R"(
     class Foo {};
     void f1();
     inline void f2() {}
   )";
   const std::string Main = R"(
     namespace {
     void ff() {} // ignore
     }
     void main_f() {} // ignore
     void f1() {}
   )";
-  runSymbolCollector(Header, Main);
+  collectSymbols(Header, Main);
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
 }
 
 TEST_F(SymbolCollectorTest, ClassMembers) {
   const std::string Header = R"(
     class Foo {
       void f() {}
       void g();
       static void sf() {}
       static void ssf();
       static int x;
     };
   )";
   const std::string Main = R"(
     void Foo::g() {}
     void Foo::ssf() {}
   )";
-  runSymbolCollector(Header, Main);
+  collectSymbols(Header, Main);
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
                                    QName("Foo::g"), QName("Foo::sf"),
                                    QName("Foo::ssf"), QName("Foo::x")));
 }
 
 TEST_F(SymbolCollectorTest, Scopes) {
   const std::string Header = R"(
     namespace na {
     class Foo {};
     namespace nb {
     class Bar {};
     }
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(QName("na"), QName("na::nb"),
                                    QName("na::Foo"), QName("na::nb::Bar")));
 }
 
 TEST_F(SymbolCollectorTest, ExternC) {
   const std::string Header = R"(
     extern "C" { class Foo {}; }
     namespace na {
     extern "C" { class Bar {}; }
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("na"), QName("Foo"),
                                             QName("na::Bar")));
 }
 
 TEST_F(SymbolCollectorTest, SkipInlineNamespace) {
   const std::string Header = R"(
     namespace na {
     inline namespace nb {
     class Foo {};
     }
     }
     namespace na {
     // This is still inlined.
     namespace nb {
     class Bar {};
     }
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(QName("na"), QName("na::nb"),
                                    QName("na::Foo"), QName("na::Bar")));
 }
 
 TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
   const std::string Header = R"(
     namespace nx {
     /// Foo comment.
     int ff(int x, double y) { return 0; }
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
           QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"),
                              ReturnType("int"), Doc("Foo comment."))));
 }
 
 TEST_F(SymbolCollectorTest, Snippet) {
   const std::string Header = R"(
     namespace nx {
     void f() {}
     int ff(int x, double y) { return 0; }
     }
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   QName("nx"),
                   AllOf(QName("nx::f"), Labeled("f()"), Snippet("f()")),
                   AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"),
                         Snippet("ff(${1:int x}, ${2:double y})"))));
 }
 
)";
   }
   auto ConcatenatedSymbols = symbolsFromYAML(ConcatenatedYAML);
   EXPECT_THAT(ConcatenatedSymbols,
               UnorderedElementsAre(QName("clang::Foo1"),
                                    QName("clang::Foo2")));
 }
 
 TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) {
-  CollectorOpts.CollectIncludePath = true;
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  CollectSymOpts.CollectIncludePath = true;
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
                                          IncludeHeader(TestHeaderURI))));
 }
 
 #ifndef _WIN32
 TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   addSystemHeadersMapping(&Includes);
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   // bits/basic_string.h$ should be mapped to <string>
   TestHeaderName = "/nasty/bits/basic_string.h";
   TestFileName = "/nasty/bits/basic_string.cpp";
   TestHeaderURI = URI::createFile(TestHeaderName).toString();
-  runSymbolCollector("class string {};", /*Main=*/"");
+  collectSymbols("class string {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("string"),
                                                   DeclURI(TestHeaderURI),
                                                   IncludeHeader("<string>"))));
 }
 #endif
 
 TEST_F(SymbolCollectorTest, STLiosfwd) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   addSystemHeadersMapping(&Includes);
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   // Symbols from <iosfwd> should be mapped individually.
   TestHeaderName = testPath("iosfwd");
   TestFileName = testPath("iosfwd.cpp");
   std::string Header = R"(
     namespace std {
       class no_map {};
       class ios {};
       class ostream {};
       class filebuf {};
     } // namespace std
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   QName("std"),
                   AllOf(QName("std::no_map"), IncludeHeader("<iosfwd>")),
                   AllOf(QName("std::ios"), IncludeHeader("<ios>")),
                   AllOf(QName("std::ostream"), IncludeHeader("<ostream>")),
                   AllOf(QName("std::filebuf"), IncludeHeader("<fstream>"))));
 }
 
 TEST_F(SymbolCollectorTest, IWYUPragma) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   PragmaHandler = collectIWYUHeaderMaps(&Includes);
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   const std::string Header = R"(
     // IWYU pragma: private, include the/good/header.h
     class Foo {};
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("Foo"), DeclURI(TestHeaderURI),
                                  IncludeHeader("\"the/good/header.h\""))));
 }
 
 TEST_F(SymbolCollectorTest, IWYUPragmaWithDoubleQuotes) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   PragmaHandler = collectIWYUHeaderMaps(&Includes);
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   const std::string Header = R"(
     // IWYU pragma: private, include "the/good/header.h"
     class Foo {};
   )";
-  runSymbolCollector(Header, /*Main=*/"");
+  collectSymbols(Header, /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("Foo"), DeclURI(TestHeaderURI),
                                  IncludeHeader("\"the/good/header.h\""))));
 }
 
 TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   Includes.addMapping(TestHeaderName, "<canonical>");
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   auto IncFile = testPath("test.inc");
   auto IncURI = URI::createFile(IncFile).toString();
   InMemoryFileSystem->addFile(IncFile, 0,
                               llvm::MemoryBuffer::getMemBuffer("class X {};"));
-  runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
-                     /*ExtraArgs=*/{"-I", testRoot()});
+  collectSymbols("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
                                          IncludeHeader("<canonical>")),
                                    AllOf(QName("Y"), DeclURI(TestHeaderURI),
                                          IncludeHeader("<canonical>"))));
 }
 
 TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   TestFileName = testPath("main.h");
   TestFileURI = URI::createFile(TestFileName).toString();
   auto IncFile = testPath("test.inc");
   auto IncURI = URI::createFile(IncFile).toString();
   InMemoryFileSystem->addFile(IncFile, 0,
                               llvm::MemoryBuffer::getMemBuffer("class X {};"));
-  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
-                     /*ExtraArgs=*/{"-I", testRoot()});
+  collectSymbols("", /*Main=*/"#include \"test.inc\"",
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
                                                   IncludeHeader(TestFileURI))));
 }
 
 TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   TestFileName = testPath("no_ext_main");
   TestFileURI = URI::createFile(TestFileName).toString();
   auto IncFile = testPath("test.inc");
   auto IncURI = URI::createFile(IncFile).toString();
   InMemoryFileSystem->addFile(IncFile, 0,
                               llvm::MemoryBuffer::getMemBuffer("class X {};"));
-  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
-                     /*ExtraArgs=*/{"-I", testRoot()});
+  collectSymbols("", /*Main=*/"#include \"test.inc\"",
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
                                                   IncludeHeader(TestFileURI))));
 }
 
 TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
-  CollectorOpts.Includes = &Includes;
+  CollectSymOpts.Includes = &Includes;
   auto IncFile = testPath("test.inc");
   auto IncURI = URI::createFile(IncFile).toString();
   InMemoryFileSystem->addFile(IncFile, 0,
                               llvm::MemoryBuffer::getMemBuffer("class X {};"));
-  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
-                     /*ExtraArgs=*/{"-I", testRoot()});
+  collectSymbols("", /*Main=*/"#include \"test.inc\"",
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
                                                   IncludeHeader(IncURI))));
 }
 
 TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   Annotations Header(R"(
     // Forward declarations of TagDecls.
     class C;
     struct S;
     union U;
 
     // Canonical declarations.
     class $cdecl[[C]] {};
     struct $sdecl[[S]] {};
     union $udecl[[U]] {int $xdecl[[x]]; bool $ydecl[[y]];};
   )");
-  runSymbolCollector(Header.code(), /*Main=*/"");
+  collectSymbols(Header.code(), /*Main=*/"");
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
           AllOf(QName("C"), DeclURI(TestHeaderURI),
                 DeclRange(Header.range("cdecl")), IncludeHeader(TestHeaderURI),
                 DefURI(TestHeaderURI), DefRange(Header.range("cdecl"))),
           AllOf(QName("S"), DeclURI(TestHeaderURI),
                 DeclRange(Header.range("sdecl")), IncludeHeader(TestHeaderURI),
                 DefURI(TestHeaderURI), DefRange(Header.range("sdecl"))),
           AllOf(QName("U"), DeclURI(TestHeaderURI),
                 DeclRange(Header.range("udecl")), IncludeHeader(TestHeaderURI),
                 DefURI(TestHeaderURI), DefRange(Header.range("udecl"))),
           AllOf(QName("U::x"), DeclURI(TestHeaderURI),
                 DeclRange(Header.range("xdecl")), DefURI(TestHeaderURI),
                 DefRange(Header.range("xdecl"))),
           AllOf(QName("U::y"), DeclURI(TestHeaderURI),
                 DeclRange(Header.range("ydecl")), DefURI(TestHeaderURI),
                 DefRange(Header.range("ydecl")))));
 }
 
 TEST_F(SymbolCollectorTest, ClassForwardDeclarationIsCanonical) {
-  CollectorOpts.CollectIncludePath = true;
-  runSymbolCollector(/*Header=*/"class X;", /*Main=*/"class X {};");
+  CollectSymOpts.CollectIncludePath = true;
+
+  collectSymbols(/*Header=*/"class X;", /*Main=*/"class X {};");
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
                            QName("X"), DeclURI(TestHeaderURI),
                            IncludeHeader(TestHeaderURI), DefURI(TestFileURI))));
 }
 
 TEST_F(SymbolCollectorTest, UTF16Character) {
   // ö is 2-bytes.
   Annotations Header(/*Header=*/"class [[pörk]] {};");
-  runSymbolCollector(Header.code(), /*Main=*/"");
+  collectSymbols(Header.code(), /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            AllOf(QName("pörk"), DeclRange(Header.range()))));
 }
 
 TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
   Annotations Header(R"(
     namespace nx {
       class $z[[Z]] {};
       class X {
         friend class Y;
         friend class Z;
         friend void foo();
         friend void $bar[[bar]]() {}
       };
       class $y[[Y]] {};
       void $foo[[foo]]();
     }
   )");
-  runSymbolCollector(Header.code(), /*Main=*/"");
+  collectSymbols(Header.code(), /*Main=*/"");
 
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   QName("nx"), QName("nx::X"),
                   AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
                   AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
                   AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
                   AllOf(QName("nx::bar"), DeclRange(Header.range("bar")))));
 }
 
 TEST_F(SymbolCollectorTest, ReferencesInFriendDecl) {
   const std::string Header = R"(
     class X;
     class Y;
   )";
   const std::string Main = R"(
     class C {
       friend ::X;
       friend class Y;
     };
   )";
-  CollectorOpts.CountReferences = true;
-  runSymbolCollector(Header, Main);
+  CollectSymOpts.CountReferences = true;
+  collectSymbols(Header, Main);
   EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), Refs(1)),
                                             AllOf(QName("Y"), Refs(1))));
 }
 
 TEST_F(SymbolCollectorTest, Origin) {
-  CollectorOpts.Origin = SymbolOrigin::Static;
-  runSymbolCollector("class Foo {};", /*Main=*/"");
+  CollectSymOpts.Origin = SymbolOrigin::Static;
+  collectSymbols("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            Field(&Symbol::Origin, SymbolOrigin::Static)));
 }
 
 TEST_F(SymbolCollectorTest, CollectMacros) {
-  CollectorOpts.CollectIncludePath = true;
+  CollectSymOpts.CollectIncludePath = true;
   Annotations Header(R"(
     #define X 1
     #define $mac[[MAC]](x) int x
     #define $used[[USED]](y) float y;
 
     MAC(p);
   )");
   const std::string Main = R"(
     #define MAIN 1  // not indexed
     USED(t);
   )";
-  CollectorOpts.CountReferences = true;
-  CollectorOpts.CollectMacro = true;
-  runSymbolCollector(Header.code(), Main);
+  CollectSymOpts.CountReferences = true;
+  CollectSymOpts.CollectMacro = true;
+  collectSymbols(Header.code(), Main);
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
           QName("p"),
           AllOf(QName("X"), DeclURI(TestHeaderURI),
                 IncludeHeader(TestHeaderURI)),
           AllOf(Labeled("MAC(x)"), Refs(0), DeclRange(Header.range("mac"))),
           AllOf(Labeled("USED(y)"), Refs(1), DeclRange(Header.range("used")))));
 }
 
+TEST_F(SymbolCollectorTest, CollectReference) {
+  const std::string Header(R"(
+  class Foo {
+  public:
+    Foo() {}
+    Foo(int);
+  };
+  class Bar;
+  void func();)");
+
+  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;
+  })");
+
+  auto H = TestTU::withHeaderCode(Header);
+  auto HeaderSymbols = H.headerSymbols();
+  auto Foo = findSymbol(HeaderSymbols, "Foo");
+  auto Bar = findSymbol(HeaderSymbols, "Bar");
+  auto Func = findSymbol(HeaderSymbols, "func");
+
+  CollectOccurrenceOpts.Filter = SymbolOccurrenceKind::Declaration |
+                                 SymbolOccurrenceKind::Definition |
+                                 SymbolOccurrenceKind::Reference;
+  CollectOccurrenceOpts.IDs = llvm::None;
+  collectOccurrences(Header, Main.code());
+  EXPECT_THAT(
+      Symbols.findOccurrences(Foo.ID),
+      testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(
+      Symbols.findOccurrences(Bar.ID),
+      testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("bar")));
+  EXPECT_THAT(
+      Symbols.findOccurrences(Func.ID),
+      testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("func")));
+
+  CollectOccurrenceOpts.IDs = {Foo.ID};
+  collectOccurrences(Header, Main.code());
+  EXPECT_THAT(
+      Symbols.findOccurrences(Foo.ID),
+      testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(Symbols.findOccurrences(Bar.ID), testing::IsEmpty());
+  EXPECT_THAT(Symbols.findOccurrences(Func.ID), testing::IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang