sammccall added inline comments.
================
Comment at: clangd/ClangdLSPServer.h:40
+ bool BuildDynamicSymbolIndex,
+ std::unique_ptr<SymbolIndex> StaticIdx);
----------------
is there a reason ClangdServer should own this? I can imagine this complicating
life for embedders. (vs a raw pointer)
================
Comment at: clangd/CodeComplete.cpp:554
+ const SpecifiedScope &SSInfo,
+ llvm::StringRef IndexSource = "") {
CompletionItem Item;
----------------
ioeric wrote:
> Maybe `IndexSourceLabel`?
Actually, can we give this a more generic name like DebuggingLabel?
That will make things less confusing as we store slightly different info here
over time (e.g. this text will definitely change when merging happens). It also
indicates that this isn't functionally important.
If you don't mind, I'd also change the value so you just pass "G" and it gets
formatted into "[G] " by this function. That means we can easily format it
differently (e.g. hide it from the UI actually include it in the JSON as an
extension field) in a way that makes sense.
================
Comment at: clangd/CodeComplete.cpp:591
- Items->isIncomplete = !Index.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
- Items->items.push_back(indexCompletionItem(Sym, Filter, SSInfo));
- });
+ // FIXME: figure out a good algorithm to merge symbols from dynamic index and
+ // static index.
----------------
nit: this comment implies that we should do a two-way merge here between
static/dynamic index. I don't think that's the case.
We're also going want to merge with AST based results, and probably want to do
that *before* generating CompletionItems, because scoring should take signals
from all sources into account.
I'd suggest reverting this function to use just one index, and calling it twice.
Then this comment about merging would live near the call site, and cover all
three sources - it'd suggest the fix where we should be applying it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
