aaron.ballman added a comment. In D141310#4063203 <https://reviews.llvm.org/D141310#4063203>, @dblaikie wrote:
> In D141310#4062776 <https://reviews.llvm.org/D141310#4062776>, @adriandole > wrote: > >> We use `icf=all` and have encountered bugs caused by function pointer >> comparisons. > > & the savings are worth it compared to icf=safe? (given the > limitations/bugs/investment in warnings like this, etc) I guess That's the part I keep struggling with. It sounds like `icf=all` is a fundamentally broken feature; there's no way to catch all of the issues it introduces in code (you really need instrumentation for that, I'd imagine). If your code isn't impacted by the fundamental issues with the feature, then it's "safe" to use, but it sure doesn't seem like `icf=all` is something we want to *encourage* people to enable by making it seem safer than it really is. On the other hand, it exists, it's not likely to go away, and giving users SOME help is better than giving users NO help. >> It's not that noisy compiling clang (eight hits). > > Good to know - I'm surprised it's that low. > > Is there some idiom we can use/document/recommend for people to use when the > warning is a false positive? (when the user is confident the functions won't > be folded together) How would the user know the warning is a false positive in the first place? ================ Comment at: clang/test/SemaCXX/compare-function-pointer.cpp:17 -bool eq1 = a == c; // expected-error {{comparison of distinct pointer types}} -bool ne1 = a != c; // expected-error {{comparison of distinct pointer types}} +bool eq1 = a == c; // expected-error {{comparison of distinct pointer types}} expected-warning {{comparison of function pointers}} +bool ne1 = a != c; // expected-error {{comparison of distinct pointer types}} expected-warning {{comparison of function pointers}} ---------------- cjdb wrote: > It doesn't make sense to me that we would emit a warning when we're already > emitting an error. +1, this feels like we're adding more heat without any light. 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
