sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG with the understanding that we'll have to move this to be more sophisticated
later later, but it's useful to have the simple version now.
Similarly I think it's OK to add the tests lat
kuhnel updated this revision to Diff 367738.
kuhnel added a comment.
addressed some review comments
question of integration with main functionality still open.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108119/new/
https://reviews.llvm.org/D108
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+ if (Opts.IndexStandardLibrary) {
+auto SLIndex = indexStandardLibrary(TFS);
+if (!SLIndex) {
A couple of issues with indexing here and placing it in the stack:
kuhnel added inline comments.
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+ if (Opts.IndexStandardLibrary) {
+auto SLIndex = indexStandardLibrary(TFS);
+if (!SLIndex || !SLIndex->get()) {
I'm not sure if returning a Expected makes sense her
kuhnel updated this revision to Diff 366856.
kuhnel marked an inline comment as done.
kuhnel added a comment.
fixed lifecycle of StdLibIndex variable
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108119/new/
https://reviews.llvm.org/D108119
Files:
sammccall added a comment.
(First just the bug that I suspect is blocking you from testing this, design
feedback to come)
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+}
+AddIndex(SLIndex->get());
+ }
this pointer will dangle after the `i
