ilya-biryukov added inline comments.
================
Comment at: clangd/index/Index.cpp:139
+ std::sort(Occurrences.begin(), Occurrences.end(),
+ [](const SymbolOccurrence &L, const SymbolOccurrence &R) {
+ return L < R;
----------------
NIT: remove the lambda? using `<` is the default.
================
Comment at: clangd/index/Index.cpp:145
+ [](const SymbolOccurrence &L, const SymbolOccurrence &R) {
+ return L == R;
+ }),
----------------
NIT: remove the lambda? Using `==` is the default.
================
Comment at: clangd/index/Index.cpp:158
+raw_ostream &operator<<(raw_ostream &OS, SymbolOccurrenceKind K) {
+ OS << static_cast<unsigned>(K);
+ return OS;
----------------
Is this used for debugging?
In that case maybe consider having a user-readable representation instead of
the number?
================
Comment at: clangd/index/Index.h:46
+
+inline bool operator==(const SymbolLocation::Position &L,
+ const SymbolLocation::Position &R) {
----------------
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.
================
Comment at: clangd/index/Index.h:349
llvm::DenseMap<SymbolID, size_t> SymbolIndex;
+
+ llvm::DenseMap<SymbolID, std::vector<SymbolOccurrence>> SymbolOccurrences;
----------------
Maybe add a comment or remove the empty line?
================
Comment at: clangd/index/Index.h:350
+
+ llvm::DenseMap<SymbolID, std::vector<SymbolOccurrence>> SymbolOccurrences;
};
----------------
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.
================
Comment at: clangd/index/SymbolCollector.cpp:272
+ };
+ if (static_cast<unsigned>(Opts.Filter) & Roles) {
+ if (auto ID = getSymbolID(D)) {
----------------
NIT: maybe use early exits and inverted conditions to keep the nesting down?
================
Comment at: clangd/index/SymbolCollector.cpp:321
+
+ const SymbolCollector::Options &CollectorOpts;
+ const SymbolCollector::Options::CollectSymbolOptions &Opts;
----------------
If we any `Options` here, why have an extra `CollectorSymbolOptions`?
================
Comment at: clangd/index/SymbolCollector.h:59
+ // If none, all symbol occurrences will be collected.
+ llvm::Optional<llvm::DenseSet<SymbolID>> IDs = llvm::None;
+ };
----------------
Could you elaborate on what this option will be used for? How do we know in
advance which symbols we're interested in?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
