[clang] [clang-tools-extra] [Clang] [Sema] Make -Wreturn-type an error by default (PR #131207)

[Aaron Ballman via cfe-commits]

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