https://github.com/AaronBallman requested changes to this pull request.
My primary concern remains the false positives with the analysis; that means we will be rejecting (a lot of) valid code. I just went through the bug database and resolved ~10 issues as duplicates of https://github.com/llvm/llvm-project/issues/111509. Some cases include: https://godbolt.org/z/z6eK9ocbb I think this change will reject a lot of valid code and actually reduce the utility of the diagnostic because people will disable the diagnostic entirely. Making it a downgradeable diagnostic helps a bit, but I think we should work on improving the analysis before we make this change. If we find we cannot reduce the false positives on fully-covered control flow, that suggests this cannot be made into a hard error (either by the implementation or by the standards committee, as WG14 is still investigating this as well: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3483.pdf) FWIW, I *want* to get to the point we can enable this by default! In the meantime, I think UBSan is the best approach to catching this class of issues as it will trigger only when you actually *do* flow off the end of a non-void function. So tl;dr: I want to accept this change but I feel like I can't, at least not without more buy-in. I would recommend running an RFC to see if the community at large is okay with moving forward with the changes. I would also love to know whether my fears are unjustified by seeing how much code breaks when trying to build packages for a distro (and I think we should do that effort rather than landing the changes and forcing Clang users to do the experiment closer to rc stages). https://github.com/llvm/llvm-project/pull/131207 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
