MosheBerman added a comment. In D123352#3475889 <https://reviews.llvm.org/D123352#3475889>, @NoQ wrote:
> I'm worried that even if the warning is correct, the suggested fix is not > necessarily the right solution. > > The very nature of path-sensitive warnings requires multiple events to happen > on the path in order for the problem to manifest. Eliminating even one of > those critical events from the path would eliminate the warning even if all > other events are still present. The fix-it hint you're adding targets only > one of those events. So it'll eliminate the warning, but so would 2-3 other > potential fixes, and there's no way to know which one is actually preferred. > > For example, you can eliminate a null-pointer-to-nonnull warning by either > changing the "to-nonnull" part (changing the attribute) or by changing the > "null-pointer" part (eg., adding a null check before the call). Both fixes > make sense depending on the circumstances. Say, if the callee function always > crashes on null pointer parameter, annotating the parameter as nullable is a > totally wrong thing to do. > > So I generally advice against fix-it hints for path-sensitive warnings. I'm > curious how you plan to deal with this problem in your specific workflow. That's a great point. The fixits are only generated for nil/null-returned, not the other nullability checks, and this is why. I actually meant to include tests for when arguments are directly returned by a code path. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
