dblaikie added a comment.

In D141310#4060069 <https://reviews.llvm.org/D141310#4060069>, @aaron.ballman 
wrote:

> In D141310#4054351 <https://reviews.llvm.org/D141310#4054351>, @dblaikie 
> wrote:
>
>> @adriandole do you plan to deploy this in a codebase? Have you tried it on a 
>> codebase already?
>>
>> I'd worry this would just be too noisy, and there's probably enough benign 
>> pointer comparisons that'll never hit the ICF false-equality situation (eg: 
>> putting some callbacks in a map/set/something - where the callbacks all do 
>> genuinely different things, so they'd never end up with accidental identical 
>> functions/folding) that it wouldn't be feasible to use this in a real 
>> codebase?
>
> Are your worries lessened by the fact that this is (by necessity of the way 
> the toolchain is composed) be an off-by-default warning that users must opt 
> into? My thinking is that this shouldn't be *too* chatty because it's 
> specific to equality comparisons between (non-nullptr) function pointers, but 
> I agree that having some confirmation about this finding true positives that 
> aren't swamped by false positives would be beneficial.

In some ways an off-by-default warning makes me worry more about it having some 
good test coverage/usage - since it won't get it incidentally from being on by 
default (of course we want on by default warnings well vetted so we don't annoy 
users - but we certainly won't be lacking feedback on them if their false 
positive rate is too high).

I'd think that there'd be enough people putting function pointers in maps or 
other data structures that would compare them that this warning would be fairly 
esoteric/only applicable to fairly small codebases (or ones that have 
incidentally avoided any function pointer usage) and may be more amenable to a 
clang-tidy check than a compiler warning.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141310/new/

https://reviews.llvm.org/D141310

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to