sammccall added inline comments.
================
Comment at: clangd/index/CMakeLists.txt:5
+
+add_clang_library(clangdIndex
+ Index.cpp
----------------
hmm, I'm not sure whether we actually want this to be a separate library. This
means we can't depend on anything from elsewhere in clangd, and may force us to
create more components.
e.g. if we want to pass contexts into the index, or if we want to reuse LSP
data models from protocol.h.
Maybe we should make this part of the main clangd lib, what do you think?
================
Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+ return Symbols.find(SymID);
----------------
hokein wrote:
> sammccall wrote:
> > assert frozen? (and in begin())
> We may not do the assert "frozen" in these getter methods -- as writers may
> also have needs to access these APIs (e.g. checking whether SymbolID is
> already in the slab before constructing and inserting a new Symbol).
Right, I'd specifically like not to allow that if possible - it makes it very
difficult to understand what the invariants are and what operations are allowed.
I think there's no such need now, right? If we want to add one in future, we
can relax this check, or add a builder, or similar - at least we should
explicitly consider it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40897
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
