sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
\o/
================
Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+ return Symbols.find(SymID);
----------------
assert frozen? (and in begin())
================
Comment at: clangd/index/Index.h:45
+ SymbolID(llvm::StringRef USR);
+ std::array<uint8_t, 20> HashValue;
+};
----------------
nit: make HashValue private?
provide operator== (and use it from DenseMapInfo).
================
Comment at: clangd/index/Index.h:108
+ static inline clang::clangd::SymbolID getEmptyKey() {
+ return clang::clangd::SymbolID("EMPTYKEY");
+ }
----------------
nit: you may want to memoize this in a local static variable, rather than
compute it each time: DenseMap calls it a lot.
================
Comment at: clangd/index/SymbolCollector.cpp:37
+ << '\n';
+ // Handle symbolic link path cases.
+ // We are trying to get the real file path of the symlink.
----------------
Can you spell out here which symbolic link cases we're handling, and what
problems we're trying to avoid?
Offline, we talked about the CWD being a symlink. But this is a different
case...
================
Comment at: clangd/index/SymbolCollector.h:35
+
+ StringRef getFilename() const {
+ return Filename;
----------------
What's this for? Seems like we should be able to handle multiple TUs with one
collector?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40897
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
