sammccall added inline comments.
================
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+ MockFSProvider FS;
----------------
ioeric wrote:
> sammccall wrote:
> > ioeric wrote:
> > > sammccall wrote:
> > > > ioeric wrote:
> > > > > Also add a test for `enqueueAll` with multiple TUs ?
> > > > Is it important to call `enqueueAll` specifically vs `enqueue` multiple
> > > > times?
> > > >
> > > > We don't have a good test fixture for a compilation database, and
> > > > `enqueueAll` is trivial...
> > > I think the randomization code worths a test.
> > >
> > > How about adding a test in ClangdServer with the auto index enabled? I
> > > think we'd also want coverage in ClangdServer anyway.
> > How would you suggest testing the randomization :-)
> >
> > The problem with a ClangdServer test is that it doesn't know anything about
> > autoindex.
> > AutoIndex lives in ClangdLSPServer, because that's where the compilation
> > database lives (because people keep cramming LSP extensions in to
> > manipulate it, and I haven't been able to remove them).
> > Nobody has worked out how to test ClangdLSPServer yet. It's a worthy
> > project, but...
> > How would you suggest testing the randomization :-)
> Not suggesting testing the behavior; just test that it works really. I guess
> a single file would also do it.
>
> > The problem with a ClangdServer test is that it doesn't know anything about
> > autoindex.
> Ah, I see. That's a bit unfortunate. ClangdServer seems to be a better place
> for the auto index. I wonder if we could move AutoIndex into ClangdServer?
> I'm not very familiar with CDB APIs, but intuitively this seems doable if we
> tweak the interface of CDB a bit e.g. inject the callback into
> `GlobalCompilationDatabase` without going through `CompilationDB`? Not sure
> if how well this would play with the CDB design though.
> ClangdServer seems to be a better place for the auto index
I fully agree.
> I wonder if we could move AutoIndex into ClangdServer? I'm not very familiar
> with CDB APIs...
Currently the global CDB is trying to be all things to all people:
- by default, it follows Tooling conventions and looks for compile_commands
above the source files
- google's hosted option wants it to be completely opaque, no indexing
- apple wants it to be an in-memory store mutated over LSP
- ericsson wants it to be a fixed directory set by the user
And we've implemented these in ad-hoc ways, not behind any real abstraction. I
don't think adding more functionality to the interface without addressing this
is a great idea, and I don't really know how to untangle all this without
asking people to rethink their use cases. But I'll think about this some more.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53032
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
