Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood marked 8 inline comments as done. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24 @@ +23,3 @@ + functionDecl(isDefinition(), returns(asString("void")), + has(compoundStmt(hasAnySubstatement(returnStmt() +

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

kimgr added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { LegalizeAdulthood wrote: > kimgr wrote: > > What happens to guard clauses invoking

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

kimgr added a subscriber: kimgr. kimgr added a comment. Came up with another test case. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { What happens to guard

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { kimgr wrote: > What happens to guard clauses invoking void functions? >

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { kimgr wrote: > LegalizeAdulthood wrote: > > kimgr wrote: > > > What

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added a comment. I would like to see some additional tests to ensure that this does not turn dead code into live code with the fix. e.g., void g(); void f() { return; g(); // This would become live code if return were

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.h:26 @@ +25,3 @@ +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html +class RedundantReturnCheck : public ClangTidyCheck { +public:

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

alexfh added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.h:26 @@ +25,3 @@ +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html +class RedundantReturnCheck : public ClangTidyCheck { +public:

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

alexfh added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:36 @@ +35,3 @@ +checkRedundantReturn(Result, Fn); + } else if (const auto *For = Result.Nodes.getNodeAs("for")) { +checkRedundantContinue(Result, dyn_cast(For->getBody()));

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24 @@ +23,3 @@ + functionDecl(isDefinition(), returns(asString("void")), + has(compoundStmt(hasAnySubstatement(returnStmt() + .bind("fn"),

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24 @@ +23,3 @@ + functionDecl(isDefinition(), returns(asString("void")), + has(compoundStmt(hasAnySubstatement(returnStmt() + .bind("fn"),

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added a comment. I'm starting with stuff that I see in code bases where I work; chances are that other people see them too. In the future, I'll look in the bug database for stuff that is similar or identical to what I'm doing. http://reviews.llvm.org/D16259

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

Eugene.Zelenko added a comment. In http://reviews.llvm.org/D16259#328834, @LegalizeAdulthood wrote: > I didn't know about the bug reports. I created this check because I have > encountered such redundant control flow in real code bases. Bug report was reflection on my code base, but I also

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood updated this revision to Diff 45110. LegalizeAdulthood added a comment. Extend this check to handle redundant `continue` statements in loop bodies. With this extension, I'm wondering if the check should be renamed redundant-control-flow instead of redundant-return. Comments?

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added a comment. I didn't know about the bug reports. I created this check because I have encountered such redundant control flow in real code bases. This would close 21984 , but not 22416

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Thank you for this check! https://llvm.org/bugs/show_bug.cgi?id=21984 waited to be implemented for so looong time :-) By the word, other return related readability check idea:

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:46-49 @@ +45,6 @@ + auto ReturnRange = CharSourceRange::getCharRange( + Start, Lexer::findLocationAfterToken( + Return->getLocEnd(), tok::semi,