ilya-biryukov added inline comments.
================
Comment at: clangd/index/SymbolCollector.cpp:95
+ auto FileName = SM.getFilename(findNameLoc(ND));
+ if (FileName.endswith(".proto.h") || FileName.endswith(".pb.h")) {
+ auto Name = ND->getName();
----------------
NIT: reduce nesting by inverting if condition (see
[[https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code|LLVM
style guide]])
================
Comment at: clangd/index/SymbolCollector.cpp:155
+ if (isPrivateProtoSymbol(ND, ASTCtx->getSourceManager()))
+ return true;
----------------
Maybe add a comment mentioning that we remove internal symbols from protobuf
code generator here?
I can easily decipher what happens here, because I know of protos, but I can
imagine the comment might be very helpful to someone reading the code and being
unfamiliar with protos.
================
Comment at: clangd/index/SymbolCollector.cpp:156
+ if (isPrivateProtoSymbol(ND, ASTCtx->getSourceManager()))
+ return true;
+
----------------
We should also run the same code to filter our those decls coming from sema
completions.
Otherwise, they might still pop up in completion results if internal proto
symbols were deserialized from preamble by clang before running the code
completion.
================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:713
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"),
+ QName("nx::Kind"),
----------------
Maybe also test that the same symbol is not excluded in the file that does not
end with `.proto.h`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46751
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
