Index: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -19,20 +19,29 @@ namespace readability { void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { - // FIXME: Support continue, break and throw. + const auto ControlFlowInterruptorMatcher = + stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"), + breakStmt().bind("break"), cxxThrowExpr().bind("throw"))); Finder->addMatcher( - compoundStmt( - forEach(ifStmt(hasThen(stmt(anyOf(returnStmt(), - compoundStmt(has(returnStmt()))))), - hasElse(stmt().bind("else"))) - .bind("if"))), + stmt(forEach( + ifStmt(hasThen(stmt( + anyOf(ControlFlowInterruptorMatcher, + compoundStmt(has(ControlFlowInterruptorMatcher))))), + hasElse(stmt().bind("else"))) + .bind("if"))), this); } void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { const auto *If = Result.Nodes.getNodeAs("if"); SourceLocation ElseLoc = If->getElseLoc(); - DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return"); + std::string ControlFlowInterruptor; + for (const auto *BindingName : {"return", "continue", "break", "throw"}) + if (Result.Nodes.getNodeAs(BindingName)) + ControlFlowInterruptor = BindingName; + + DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'") + << ControlFlowInterruptor; Diag << tooling::fixit::createRemoval(ElseLoc); // FIXME: Removing the braces isn't always safe. Do a more careful analysis. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst @@ -3,7 +3,62 @@ readability-else-after-return ============================= +`LLVM Coding Standards `_ advises to +reduce indentation where possible and where it makes understanding code easier. +Early exit is one of the suggested enforcements of that. Please do not use +`else` or `else if` after something that interrupts control flow - like +`return`, `break`, `continue`, `throw`. -Flags the usages of ``else`` after ``return``. +The following piece of code illustrates how the check works. This piece of code: -http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return +.. code-block:: c++ + + void foo(int Value) { + int Local = 0; + for (int i = 0; i < 42; i++) { + if (Value == 1) { + return; + } else { + Local++; + } + + if (Value == 2) + continue; + else + Local++; + + if (Value == 3) { + throw 42; + } else { + Local++; + } + } + } + + +Would be transformed into: + +.. code-block:: c++ + + void foo(int Value) { + int Local = 0; + for (int i = 0; i < 42; i++) { + if (Value == 1) { + return; + } + Local++; + + if (Value == 2) + continue; + Local++; + + if (Value == 3) { + throw 42; + } + Local++; + } + } + + +This checks helps to enforce this `Coding Standars recommendation +`_. Index: clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp @@ -3,16 +3,16 @@ void f(int a) { if (a > 0) return; - else // comment -// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return -// CHECK-FIXES: {{^}} // comment + else // comment-0 + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-0 return; if (a > 0) { return; - } else { // comment -// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return -// CHECK-FIXES: } // comment + } else { // comment-1 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} } // comment-1 return; } @@ -28,7 +28,34 @@ f(0); else if (a > 10) return; - else + else // comment-2 + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-2 f(0); } +void foo() { + for (unsigned x = 0; x < 42; ++x) { + if (x) { + continue; + } else { // comment-3 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue' + // CHECK-FIXES: {{^}} } // comment-3 + x++; + } + if (x) { + break; + } else { // comment-4 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break' + // CHECK-FIXES: {{^}} } // comment-4 + x++; + } + if (x) { + throw 42; + } else { // comment-5 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw' + // CHECK-FIXES: {{^}} } // comment-5 + x++; + } + } +}