[PATCH] D141310: [clang] add -Wcompare-function-pointers

Tue Jan 17 13:59:41 PST 2023

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