malaperle added a comment. In https://reviews.llvm.org/D46751#1099786, @sammccall wrote:
> In https://reviews.llvm.org/D46751#1099633, @malaperle wrote: > > > In https://reviews.llvm.org/D46751#1099235, @sammccall wrote: > > > > > @malaperle to expand on what Eric said, adding proto hacks with false > > > positives and no way to turn them off is indeed not the way to go here! > > > There's probably going to be other places we want to filter symbols too, > > > and it should probably be extensible/customizable in some way. > > > We don't yet have enough examples to know what the structure should be > > > (something regex based, a code-plugin system based on `Registry`, or > > > something in between), thus the simplest/least invasive option for now > > > (it's important for our internal rollout to have *some* mitigation in > > > place). > > > @ioeric can you add a comment near the proto-filtering stuff indicating > > > we should work out how to make this extensible? > > > > > > I agree with all of that. What I don't quite understand is why a flag is > > not ok? Just a fail-safe switch in the mean time? You can even leave it on > > by default so your internal service is not affected. > > > I think a flag doesn't solve much of the problem, and adds new ones: > > - users have to find the flag, and work out how to turn it on in their > editor, and (other than embedders) few will bother. And each flag hurts the > usability of all the other flags. > - if this heuristic is usable only sometimes, that's at codebase granularity, > not user granularity. Flags don't work that way. (Static index currently has > this problem...) > - these flags end up in config files, so if we later remove the flag we'll > *completely* break clangd for such users I don't really agree with those points, but... >> We know for a fact that some code bases like Houdini won't work with this, >> at least there will be an option to make it work. > > Is this still the case after the last revision (with the comment check?) > Agree we should only hardwire this on if we are confident that false > positives are vanishingly small. ...I hadn't noticed the latest version. I think it's safe enough in the new version that we don't need to discuss this much further until it becomes a bigger problem (more libraries, etc). 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
