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

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 11 07:40:56 PDT 2018


malaperle added a comment.

In https://reviews.llvm.org/D46751#1095923, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D46751#1095894, @malaperle wrote:
>
> > Can there be an option for this? This seems very library specific and could break other code bases. Ideally, there would be a generic mechanism for this kind of filtering, i.e. specify a pattern of excluded files or symbol names. But I understand this would be cumbersome because you want to filter only *some* symbol names in *some* files, so it would be difficult for users to specify this intersection of conditions on command-line arguments, for example. I think this needs to be discussed a bit more or have this turned off by default (with an option to turn on!) until there is a more general solution for this kind of filtering.
>
>
> Having user-configurable filtering may certainly be useful, but requires some design to get right.
>  And even if we have it, I think there's value in automatically handling popular frameworks, unless we know it might break other code **in practice**.


Here :)
http://www.sidefx.com/docs/hdk/_s_o_p___bone_capture_lines_8proto_8h_source.html

> I feel it's better to have the filtering for protos **enabled** by default

I like the idea that things work "out of the box", we have to make sure that it doesn't make it buggy for certain code bases and impossible to work around.

In https://reviews.llvm.org/D46751#1095926, @ilya-biryukov wrote:

> So, the first line of the file generated by proto compiler seems to be something like this:
>
>   // Generated by the protocol buffer compiler.  DO NOT EDIT!
>
> If we check the symbol comes from a file with this comment, there will be zero chance that we guess it wrong.
>  And we can always filter, in addition to the user-provided filters that @malaperle proposed (which also sound like a very useful feature to me!)
>
> @ioeric, @malaperle, @sammccall, WDYT?


I think this is good if that's true that the comment is always there. I think it would be OK for this to be enabled by default, with a general option to turn heuristics off. Not sure what to call it... `-use-symbol-filtering-heuristics` :)

If handling for other libraries is added later it would be good to split out this code a bit. A collection of "filters" could be passed to symbol collector. Each filter/framework-handling could be in it's own source file. Later...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46751





More information about the cfe-commits mailing list