[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D40897#946911, @hokein wrote:

> Our rough plan would be
>
> 1. Define the Symbol structure.
> 2. Design the interfaces of SymbolIndex, ASTIndex.
> 3. Combine 1) and 2) together to make global code completion work (we'd use YAML dataset for LLVM project, note that this is not a final solution, it would be hidden in an `--experimental` flag).
> 4. Switch to use the dataset from index-while-building when it is ready.


Thanks for the explanation. On my end, the plan is not quite sequential. The branch I am developing has interfaces for querying, building and a dataset format, let's call it ClangdIndexDataStorage, which is different from index-while-building (libIndexStore). I also have a version that uses libIndexStore through the same interfaces. So with that in mind, there are too main activities:

1. Work towards the interfaces for using the index (this patch, and Eric's). From my perspective, I will make sure that it can be as compatible as possible with reading the index from disk and the features we want to develop. One important aspect is to have a good balance between memory and performance. In Eclipse CDT and also the branch I work on using ClangdIndexDataStorage, the emphasis was to minimize memory consumption and have a configurable cache size. But different choices could be made here, perhaps I can start a discussion about that separately.
2. Work on index-while-building or the other format getting adopted in Clangd. The index-while-building (libIndexStore) is promising but also has a few missing pieces. We need a mapping solution (LMDB equivalent). We also need to make sure it's fast enough and contain enough information for the features we will need, etc.



================
Comment at: clangd/Symbol.h:23
+  // The path of the source file where a symbol occurs.
+  std::string FilePath;
+  // The offset to the first character of the symbol from the beginning of the
----------------
sammccall wrote:
> ioeric wrote:
> > Is this relative or absolute?
> Having every symbol own a copy of the filepath seems wasteful.
> It seems likely that the index will have per-file information too, so this representation should likely be a key to that. Hash of the filepath might work?
How we model it is that a symbol doesn't have a "location", but its occurrence do. One could consider the location of a symbol to be either its declaration occurrence (SymbolRole::Declaration) or its definition (SymbolRole::Definition).
What we do to get the location path is each occurrence has a pointer (a "database" pointer, but it doesn't matter) to a file entry and then we get the path from the entry.

So conceptually, it works a bit like this (although it fetches information on disk).
```
class IndexOccurrence {
IndexOccurrence *FilePtr;

std::string Occurrence::getPath() {
  return FilePtr->getPath();
}
};
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897