ioeric added a comment.
Thanks for sharing the example Marc! It's a bit surprising to see files that
are not protobuf-generated named proto.h.
I'm not a big fan of parsing file comment in proto. It seems a bit cumbersome
and we might not be able (or too expensive) to do so for completion results
from sema (if we do want to filter at completion time).
Pattern-based filtering could be an option as it wouldn't require code
modification and could support potentially more filters, although I'm a bit
worries about rules getting too complicated (e.g. filters on symbol kinds etc)
or running into limitation.
But for now, it seems to me that the easiest approach is putting an option
around proto heuristic for the symbol collector, until we need more filters.
================
Comment at: clangd/index/SymbolCollector.cpp:156
+ if (isPrivateProtoSymbol(ND, ASTCtx->getSourceManager()))
+ return true;
+
----------------
ilya-biryukov wrote:
> 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.
Just want to clarify before going further. IIUC, in *index-based* completion,
the preamble could still contain symbols from headers such that sema completion
could still give you symbols from headers.
If we do need to build the filter into code completion, we would need more
careful design as code completion code path is more latency sensitive.
================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:713
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"),
+ QName("nx::Kind"),
----------------
ilya-biryukov wrote:
> Maybe also test that the same symbol is not excluded in the file that does
> not end with `.proto.h`?
Sounds good.
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
