njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed.
This check, more specifically the fixes, seem quite dangerous. Changing function signatures is highly likely to cause compilation issues. For a starters this check doesn't appear to examine call sites of the function to ensure they don't use the return value. There's also the case where the function is used as a callback, this can't change the signature as the callback type would then mismatch. From these 2 issues, this shouldn't trigger on functions with external linkage. There's the case where the return value is based on some preprocessor constant. You are showing how it handles NULL and EXIT_SUCCESS, however what if the macro is called BUILT_WITH_FEATURE_X, that's likely a build setting and shouldn't be triggered upon. Obviously there's no hard and fast way to determine what's a dependent macro and what isn't, so either never trigger on returns where the value is a macro, or have a list of allowed macros, defaulted to containing the likes of NULL. With all being said, I feel that while the diagnosing of these functions can have some value, an automated fix probably isn't a good idea. 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
