hokein marked an inline comment as done.
hokein added inline comments.
================
Comment at: clangd/index/CMakeLists.txt:5
+
+add_clang_library(clangdIndex
+ Index.cpp
----------------
sammccall wrote:
> 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?
As discussed offline, made it to the main clangd library.
================
Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+ return Symbols.find(SymID);
----------------
sammccall wrote:
> 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.
hmm, SymbolCollector has such need at the moment -- ` if (Symbols.find(ID) !=
Symbols.end()) return true;` to avoid creating a duplicated Symbol.
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
