kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/unittests/IndexActionTests.cpp:22
using ::testing::ElementsAre;
+using testing::EndsWith;
using ::testing::Not;
----------------
nit `using ::testing::EndsWith`
was this `add using` tweak ?
================
Comment at: clang-tools-extra/clangd/unittests/IndexActionTests.cpp:261
+ addFile(testPath("good.h"), R"cpp(
+ struct S { int s; };
+ void f1() { S s; }
----------------
maybe give the member a different name to distinguish it from the local var in
function ?
================
Comment at: clang/include/clang/Index/IndexingOptions.h:40
+ // This prevents traversal, so skipping a struct means its declaration an
+ // members won't be indexed, but references that struct elsewhere will be.
+ // Currently this is only checked for top-level declarations.
----------------
s/references that/references to that
================
Comment at: clang/lib/Index/IndexingAction.cpp:142
+ ShouldSkipFunctionBody =
+ [ShouldTraverseDecl(Opts.ShouldTraverseDecl)](const Decl *D) {
+ return !ShouldTraverseDecl(D);
----------------
are we happy with the copy here ?
I am a little uneasy, as these functions can preserve state. Currently the one
used in here and the one used in `IndexingContext::indexTopLevelDecl` are
different copies.
Maybe rather progate a null function into the `IndexASTConsumer` and create
this default lambda in there, making use of the function inside
`IndexingContext::IndexOpts` without a copy?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80296/new/
https://reviews.llvm.org/D80296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
