aaron.ballman added a comment. In D140860#4031872 <https://reviews.llvm.org/D140860#4031872>, @dblaikie wrote:
> The risk now is that this might significantly regress/add new findings for > this warning that may not be sufficiently bug-finding to be worth immediate > cleanup, causing users to have to choose between extensive lower-value > cleanup and disabling the warning entirely. > > Have you/could you run this over a significant codebase to see what sort of > new findings the modified warning finds, to see if they're high quality bug > finding, or mostly noise/check for whether this starts to detect certain > idioms we want to handle differently? > > It might be hard to find a candidate codebase that isn't already > warning-clean with GCC (at least Clang/LLVM wouldn't be a good candidate > because of this) & maybe that's sufficient justification to not worry too > much about this outcome... > > @aaron.ballman curious what your take on this might be Thank you for the ping (and the patience waiting on my response)! I think there's a design here that could make sense to me. Issuing the diagnostic when there is a literal is silly because the literal value is never going to change. However, with a constant expression, the value could change depending on configuration. This begs the question of: what do we do with literals that are expanded from a macro? It looks like we elide the diagnostic in that case, but macros also imply potential configurability. So I think the design that would make sense to me is to treat macro expansions and constant expressions the same way (diagnose) and only elide the diagnostic when there's a (possibly string) literal. WDYT? I would also like to see the diagnostic run over some reasonably large corpus of code as it may point out cases where our reasoning is incorrect that we can address. But I don't insist on it in this case given that this is also coming more in line with GCC's behavior (I've not tested how they handle the macro-expanding-to-a-literal case though). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140860/new/ https://reviews.llvm.org/D140860 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
