[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

[Ilya Biryukov via Phabricator via cfe-commits]

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