aaron.ballman added a comment. In D96082#2561806 <https://reviews.llvm.org/D96082#2561806>, @steveire wrote:
> In D96082#2550468 <https://reviews.llvm.org/D96082#2550468>, @LukasHanel > wrote: > >> In D96082#2549943 <https://reviews.llvm.org/D96082#2549943>, @steveire wrote: >> >>> In D96082#2545807 <https://reviews.llvm.org/D96082#2545807>, @LukasHanel >>> wrote: >>> >>>> Hi, thanks for discussing my proposal! >>>> >>>> - Usefulness of the fix-it's >>> >>> I agree with @njames93 that this check is dangerous. Even if you extended >>> it to port callExprs, that would only work in translation units which can >>> see the definition. >> >> Are you saying I should just remove the fix-its altogether? >> Or, put them under some option that is off by default? > > I'm not sure this check meets the general applicability and usefulness > threshold to be part of clang-tidy. The warning alone isn't actionable. > Someone wanting to take action on the warning would have to write their own > clang tidy check to make the change across their codebase. Add that to the > false-positive issues I mentioned before. > > Does anyone have any other thoughts? I agree. I'm not certain such a check could ever be generally applicable. For example, some functions return zero always because it's part of the API design. e.g., struct Base { virtual int foo() { return 0; /* Degenerate case */ } }; struct Derived : Base { int foo() override; // More useful implementation }; // Or namespace std { template <> class numeric_limits<MyAwesomeType> { public: ... static constexpr MyAwesomeType lowest() noexcept { return 0; /* implicit conversion to the return type */ } ... }; Also, there's nothing particularly interesting about `0` as opposed to any other constant return value. A somewhat similar check that would be interesting is a function that returns the same value on all control paths, as that may signify a logical error on the part of the programmer. e.g., int foo() { if (whatever) return 0; ... code without return statements ... return 0; } // Or int bar() { if (whatever) return foo(); else ... ... return foo(); } but I'd want this check to ignore functions that only have a single `return` statement in them in order to reduce noise. 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
