LukasHanel added a comment. In D96082#2566984 <https://reviews.llvm.org/D96082#2566984>, @steveire wrote:
> In D96082#2565339 <https://reviews.llvm.org/D96082#2565339>, @aaron.ballman > wrote: > >> A somewhat similar check that would be interesting is a function that >> returns the same value on all control paths > > I think we shouldn't try to design a new, different check in the comments of > this MR. > > I think it would be better to finalize what to do with this one. > > Request further work to also change expressions in all affected TUs? Or close? Hi, I finally managed to run this checker on my own code base and I run into all the above problems: 1. There are some sloppy areas in the code that this checker nicely highlights. 2. Many false-positives for example with callbacks or involving preprocessor 3. Some questionable suggestions, like where it is breaking code-symmetry with a set of handlers that have the same signatures but some always return 0. In conclusion for this checker: - it's a good way to spot sloppy areas in the code. - You couldn't run it in -Werror mode to enforce clean code. - Yes, I could filter more FP/noise, e.g. functions that have a single return statement in the body; plus maybe assert. - However, if you have a policy to use the return value of all functions, this checker can be a good way to clean the code first. I am ok to close this one, I will park it somewhere. In the meantime, I try to spin up some fixes for the false positives that I see with other checkers, as mentioned above. As a follow-up to this checker, clang -Wdocumentation does not understand the `@retval` command. I was thinking of adding a new clang-tidy checker for this to verify/complete the list of documented return values. In this case where the documentation does not say `@retval 0 always`, than I will come back to the checker here, suggest make the function void or add the "always" to the text :). The work with the useless-return-value was a study towards this new @retval checker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96082/new/ https://reviews.llvm.org/D96082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
