njames93 updated this revision to Diff 449048.
njames93 added a comment.
Rebase and Ping??
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130181/new/
https://reviews.llvm.org/D130181
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-split.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
@@ -0,0 +1,164 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits -format-style=llvm %t -- \
+// RUN: -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2}}"
+
+// Run the check with the braces around statements check also enabled, but
+// ShortStatementLines is set so that we shouldn't add braces.
+// RUN: %check_clang_tidy %s readability-use-early-exits,readability-braces-around-statements -format-style=llvm %t -- \
+// RUN: -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2, \
+// RUN: readability-braces-around-statements.ShortStatementLines: 2}}"
+
+// Just to hit the threshold
+void padLines();
+
+void nomralFunc(int *A) {
+ // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+ if (A) {
+ *A = 10;
+ }
+ // CHECK-FIXES: if (!A)
+ // CHECK-FIXES-NEXT: return;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: *A = 10;
+}
+
+void funcWithTrailing(int *B) {
+ // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+ if (B) {
+ *B = 10;
+ }
+ return;
+ ;
+ // CHECK-FIXES: if (!B)
+ // CHECK-FIXES-NEXT: return;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: *B = 10;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: return;
+ // CHECK-FIXES-NEXT: ;
+}
+
+void normal(int A, int B) {
+
+ while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+ if (A == B) {
+ padLines();
+ }
+ }
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (A != B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+
+ // Eat continue and empty nulls
+ while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+ if (A != B) {
+ padLines();
+ }
+ continue;
+ ;
+ continue;
+ }
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (A == B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-NEXT: ;
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-NEXT: }
+}
+
+void toShort(int A, int B) {
+ while (true) {
+ if (A == B) {
+ }
+ }
+}
+
+void hasElse(int A, int B) {
+ while (true) {
+ if (A == B) {
+
+ } else {
+ }
+ }
+}
+
+void hasTrailingStmt(int A, int B) {
+ while (true) {
+ if (A == B) {
+ }
+ padLines();
+ }
+}
+
+void nested(int A, int B) {
+ // if (A > B) {
+ // CHECK-MESSAGES: [[@LINE+6]]:5: warning: use early exit
+ // if (B < A) {
+ // CHECK-MESSAGES: [[@LINE+5]]:7: warning: use early exit
+ // if (A == B) {
+ // CHECK-MESSAGES: [[@LINE+4]]:9: warning: use early exit
+ while (true) { // Nested
+ if (A > B) {
+ if (B < A) {
+ if (A != B) {
+ padLines();
+ }
+ }
+ }
+ } // EndLoop
+ // CHECK-FIXES: while (true) { // Nested
+ // CHECK-FIXES-NEXT: if (A <= B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (B >= A)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (A == B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: } // EndLoop
+}
+
+void badNested(bool B) {
+ // Ensure check doesn't check for nested `if` if outer `if` has else.
+ while (true) {
+ if (B) {
+ if (B) {
+ }
+ } else {
+ }
+ }
+}
+
+void semiNested(int A, int B) {
+ // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+ while (true) { // SemiNested
+ if (A > B) {
+ if (B < A) {
+ if (A != B) {
+ padLines();
+ }
+ } else {
+ }
+ }
+ }
+ // CHECK-FIXES: while (true) { // SemiNested
+ // CHECK-FIXES-NEXT: if (A <= B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (B < A) {
+ // CHECK-FIXES-NEXT: if (A != B) {
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: } else {
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-split.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-split.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits -format-style=llvm %t -- \
+// RUN: -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2, \
+// RUN: readability-use-early-exits.SplitConjunctions: true}}"
+
+void padLines();
+
+bool A, B, C;
+
+void conjunction() {
+ // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+ while (true) {
+ if (A && B) {
+ padLines();
+ }
+ }
+
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (!A)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (!B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+
+ // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+ while (true) {
+ if (A && B && !C) {
+ padLines();
+ }
+ }
+
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (!A)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (!B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (C)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+
+ // CHECK-MESSAGES: [[@LINE+3]]:5: warning: use early exit
+ // CHECK-MESSAGES: [[@LINE+3]]:7: warning: use early exit
+ while (true) {
+ if (A && B) {
+ if (B && !C) {
+ padLines();
+ }
+ }
+ }
+
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (!A)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (!B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (!B)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: if (C)
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+}
+
+void disjunction() {
+ // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+ while (true) {
+ if (A || B) {
+ padLines();
+ }
+ }
+
+ // CHECK-FIXES: while (true) {
+ // CHECK-FIXES-NEXT: if (!(A || B))
+ // CHECK-FIXES-NEXT: continue;
+ // CHECK-FIXES-EMPTY:
+ // CHECK-FIXES-NEXT: padLines();
+ // CHECK-FIXES-NEXT: }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits,google-readability-braces-around-statements -format-style=llvm %t -- \
+// RUN: -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2, \
+// RUN: google-readability-braces-around-statements.ShortStatementLines: 0}}"
+
+// RUN: %check_clang_tidy %s readability-use-early-exits,hicpp-braces-around-statements -format-style=llvm %t -- \
+// RUN: -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2}}"
+void nomralFunc(int *A) {
+ // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+ if (A) {
+ *A = 10;
+ }
+ // CHECK-FIXES: if (!A) {
+ // CHECK-FIXES-NEXT: return;
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: *A = 10;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
@@ -0,0 +1,108 @@
+.. title:: clang-tidy - readability-use-early-exits
+
+readability-use-early-exits
+===========================
+
+Finds ``if`` statements inside functions and loops that could be flipped to
+make an early exit.
+
+Example:
+
+.. code-block:: c++
+
+ void Process(MyClass* C) {
+ if (C) {
+ // Do some long processing.
+ }
+ }
+
+ void Process(span<MyClass*> C){
+ for (MyClass *Item : C) {
+ if (Item) {
+ // Do some long processing.
+ }
+ }
+ }
+
+Would be transformed to:
+
+.. code-block:: c++
+
+ void Process(MyClass* C) {
+ if (!C)
+ return;
+ // Do some long processing.
+ }
+
+ void Process(span<MyClass*> C){
+ for (MyClass *Item : C) {
+ if (!Item)
+ continue;
+ // Do some long processing.
+ }
+ }
+
+Options
+-------
+
+.. option:: LineCountThreshold
+
+ Don't transform any ``if`` statement if the statement uses less than this
+ many lines. Default value is `10`.
+
+.. option:: SplitConjunctions
+
+ If `true`, split up conditions with cunjunctions (``&&``) into multiple
+ ``if`` statements. Default value is `false`.
+
+ When `true`:
+
+ .. code-block:: c++
+
+ void Process(bool A, bool B) {
+ if (A && B) {
+ // Long processing.
+ }
+ }
+
+ Would be transformed to:
+
+ .. code-block:: c++
+
+ void Process(bool A, bool B) {
+ if (!A)
+ return;
+
+ if (!B)
+ return;
+
+ // Long processing.
+ }
+
+.. option:: WrapInBraces
+
+ If `true`, the early exit will be wrapped in braces.
+
+ If unspecified, the value will be inferred based on whether
+ :doc:`readability-braces-around-statements <braces-around-statements>` or one
+ of its alias' are active.
+
+ When `true`, The above exmples would be transformed to:
+
+ .. code-block:: c++
+
+ void Process(MyClass *C) {
+ if (!C) {
+ return;
+ }
+ // Do some long processing
+ }
+
+ void Process(span<MyClass*> C){
+ for (MyClass *Item : C) {
+ if (!Item) {
+ continue;
+ }
+ // Do some long processing.
+ }
+ }
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -360,6 +360,7 @@
`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release.html>`_, "Yes"
`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix.html>`_, "Yes"
`readability-use-anyofallof <readability/use-anyofallof.html>`_,
+ `readability-use-early-exits <readability/use-early-exits.html>`_, "Yes"
`zircon-temporary-objects <zircon/temporary-objects.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,12 @@
New checks
^^^^^^^^^^
+- New :doc:`readability-use-early-exits
+ <clang-tidy/checks/readability/use-early-exits>` check.
+
+ Finds ``if`` statements inside functions and loops that could be flipped to
+ make an early exit.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
@@ -0,0 +1,44 @@
+//===--- UseEarlyExitsCheck.h - clang-tidy ----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEEARLYEXITSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEEARLYEXITSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds `if` statements inside functions and loops that could be flipped to
+/// make an early exit.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/use-early-exits.html
+class UseEarlyExitsCheck : public ClangTidyCheck {
+public:
+ UseEarlyExitsCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+ unsigned getLineCountThreshold() const { return LineCountThreshold; }
+ bool getSplitConjunctions() const { return SplitConjunctions; }
+ bool getWrapInBraces() const { return WrapInBraces; }
+
+private:
+ const unsigned LineCountThreshold;
+ const bool SplitConjunctions;
+ const bool WrapInBraces;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEEARLYEXITSCHECK_H
Index: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
@@ -0,0 +1,294 @@
+//===--- UseEarlyExitsCheck.cpp - clang-tidy ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseEarlyExitsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static bool needsParensAfterUnaryNegation(const Expr *E) {
+ E = E->IgnoreImpCasts();
+ if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
+ return true;
+
+ if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))
+ return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
+ Op->getOperator() != OO_Subscript;
+
+ return false;
+}
+
+static void addReverseConditionFix(DiagnosticBuilder &Diag,
+ const Expr *Condition,
+ const ASTContext &Ctx) {
+ if (const auto *BO = dyn_cast<BinaryOperator>(Condition)) {
+ if (BO->isComparisonOp() && BO->getOpcode() != BO_Cmp) {
+ Diag << FixItHint::CreateReplacement(
+ BO->getOperatorLoc(),
+ BinaryOperator::getOpcodeStr(
+ BinaryOperator::negateComparisonOp(BO->getOpcode())));
+ return;
+ }
+ } else if (const auto *UO = dyn_cast<UnaryOperator>(Condition)) {
+ if (UO->getOpcode() == UO_LNot) {
+ Diag << FixItHint::CreateRemoval(UO->getOperatorLoc());
+ if (const auto *Parens = dyn_cast<ParenExpr>(UO->getSubExpr())) {
+ Diag << FixItHint::CreateRemoval(Parens->getLParen());
+ Diag << FixItHint::CreateRemoval(Parens->getRParen());
+ }
+ return;
+ }
+ } else if (const auto *BL = dyn_cast<CXXBoolLiteralExpr>(Condition)) {
+ Diag << FixItHint::CreateReplacement(BL->getSourceRange(),
+ BL->getValue() ? "false" : "true");
+ return;
+ } else if (const auto *IL = dyn_cast<IntegerLiteral>(Condition)) {
+ Diag << FixItHint::CreateReplacement(
+ IL->getSourceRange(), IL->getValue().getBoolValue() ? "0" : "1");
+ return;
+ }
+ if (needsParensAfterUnaryNegation(Condition)) {
+ Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!(")
+ << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Condition->getEndLoc(), 0,
+ Ctx.getSourceManager(),
+ Ctx.getLangOpts()),
+ ")");
+ } else {
+ Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!");
+ }
+}
+
+static void addConjunctionReverseOneStep(DiagnosticBuilder &Diag, const Expr *E,
+ const ASTContext &Ctx,
+ StringRef Replacement) {
+ const auto *BO = dyn_cast<BinaryOperator>(E);
+ if (BO && BO->getOpcode() == BO_LAnd) {
+ addConjunctionReverseOneStep(Diag, BO->getLHS(), Ctx, Replacement);
+ Diag << FixItHint::CreateReplacement(BO->getOperatorLoc(), Replacement);
+ addConjunctionReverseOneStep(Diag, BO->getRHS(), Ctx, Replacement);
+ } else {
+ addReverseConditionFix(Diag, E, Ctx);
+ }
+}
+
+static void addConjunctionReverseFix(DiagnosticBuilder &Diag,
+ const Expr *Condition,
+ const ASTContext &Ctx,
+ StringRef Continuation) {
+ const auto *BO = dyn_cast<BinaryOperator>(Condition);
+ if (!BO || BO->getOpcode() != BO_LAnd) {
+ addReverseConditionFix(Diag, Condition, Ctx);
+ return;
+ }
+ addConjunctionReverseOneStep(Diag, Condition, Ctx,
+ SmallString<32>({") ", Continuation, "\nif ("}));
+}
+
+static llvm::iterator_range<CompoundStmt::const_reverse_body_iterator>
+getBodyReverse(const CompoundStmt *S) {
+ return {S->body_rbegin(), S->body_rend()};
+}
+
+namespace {
+
+class EarlyExitVisitor : public RecursiveASTVisitor<EarlyExitVisitor> {
+ UseEarlyExitsCheck &Check;
+ ASTContext &Ctx;
+
+public:
+ EarlyExitVisitor(UseEarlyExitsCheck &Check, ASTContext &Ctx)
+ : Check(Check), Ctx(Ctx) {}
+
+ bool traverse() { return TraverseAST(Ctx); }
+
+ template <typename TerminationStmt>
+ void diagnoseIf(const IfStmt *If, StringRef Continuation);
+
+ template <typename TerminationStmt>
+ void checkBody(const CompoundStmt *CS, StringRef Continuation) {
+ for (const Stmt *Item : getBodyReverse(CS)) {
+ // If the last statement in the function is a return, ignore it.
+ if (isa<TerminationStmt>(Item))
+ continue;
+ // While were here, we can safely ignore empty null stmts.
+ if (isa<NullStmt>(Item) && !cast<NullStmt>(Item)->hasLeadingEmptyMacro())
+ continue;
+ if (const auto *If = dyn_cast<IfStmt>(Item))
+ diagnoseIf<TerminationStmt>(If, Continuation);
+ break;
+ }
+ }
+
+ bool VisitFunctionDecl(const FunctionDecl *Func) {
+ // Skip any declarations.
+ if (!Func->doesThisDeclarationHaveABody())
+ return true;
+ // Just ignore no return functions.
+ if (Func->isNoReturn())
+ return true;
+ // Early exit logic only works for functions that return void.
+ // FIXME: Explore options where following the IfStmt there is a return
+ // value with a literal return.
+ // bool hasSomething(Class *S) {
+ // if (S) {
+ // return S->hasSomething();
+ // }
+ // return false;
+ // }
+ // bool hasSomething(Class *S) {
+ // if (!S)
+ // return false;
+ // return S->hasSomething();
+ // return false; // Ideally this would be removed too.
+ // }
+ if (!Func->getReturnType()->isVoidType())
+ return true;
+ // FIXME: explore if CoreturnStmt could work here also.
+ checkBody<ReturnStmt>(cast<CompoundStmt>(Func->getBody()), "return");
+ return true;
+ }
+
+ bool VisitForStmt(const ForStmt *For) {
+ if (const auto *CS = dyn_cast_or_null<CompoundStmt>(For->getBody())) {
+ checkBody<ContinueStmt>(CS, "continue");
+ }
+ return true;
+ }
+
+ bool VisitWhileStmt(const WhileStmt *While) {
+ if (const auto *CS = dyn_cast_or_null<CompoundStmt>(While->getBody())) {
+ checkBody<ContinueStmt>(CS, "continue");
+ }
+ return true;
+ }
+
+ bool VisitDoStmt(const DoStmt *Do) {
+ if (const auto *CS = dyn_cast_or_null<CompoundStmt>(Do->getBody())) {
+ checkBody<ContinueStmt>(CS, "continue");
+ }
+ return true;
+ }
+
+ bool VisitCXXForRangeStmt(const CXXForRangeStmt *ForRange) {
+ if (const auto *CS = dyn_cast_or_null<CompoundStmt>(ForRange->getBody())) {
+ checkBody<ContinueStmt>(CS, "continue");
+ }
+ return true;
+ }
+};
+
+template <typename TerminationStmt>
+void EarlyExitVisitor::diagnoseIf(const IfStmt *If, StringRef Continuation) {
+ // Ignore const(expr|eval) if statements.
+ if (If->isConsteval() || If->isConstexpr())
+ return;
+ // We can't transform if there is an else.
+ if (If->hasElseStorage())
+ return;
+ // If there is a variable in the condition, This transformation would mean it
+ // goes out of scope before the current then branch can use it.
+ if (If->hasVarStorage())
+ return;
+ // As above, only technically if the init doesn't actually have any decls, we
+ // could still do the transformation. But thats not exactly a common idiom.
+ if (If->hasInitStorage())
+ return;
+ const auto *BodyCS = llvm::dyn_cast_or_null<CompoundStmt>(If->getThen());
+ if (!BodyCS)
+ return;
+ const SourceManager &SM = Ctx.getSourceManager();
+ SourceLocation Begin = If->getBeginLoc();
+ SourceLocation End = If->getEndLoc();
+ FileID BeginFile = SM.getFileID(Begin);
+ FileID EndFile = SM.getFileID(End);
+ if (BeginFile != EndFile)
+ return;
+ unsigned BeginLine = SM.getSpellingLineNumber(Begin);
+ unsigned EndLine = SM.getSpellingLineNumber(End);
+ if (BeginLine > EndLine)
+ return; // This case can't be good.
+ if ((EndLine - BeginLine) < Check.getLineCountThreshold())
+ return;
+ {
+ SmallString<32> C2({(Check.getWrapInBraces() ? "{\n" : "\n"), Continuation,
+ Check.getWrapInBraces() ? ";\n}" : ";\n"});
+ auto Diag = Check.diag(If->getBeginLoc(), "use early exit")
+ << FixItHint::CreateInsertion(If->getThen()->getBeginLoc(), C2)
+ << FixItHint::CreateRemoval(BodyCS->getLBracLoc())
+ << FixItHint::CreateRemoval(BodyCS->getRBracLoc());
+ if (Check.getSplitConjunctions())
+ addConjunctionReverseFix(Diag, If->getCond(), Ctx, C2);
+ else
+ addReverseConditionFix(Diag, If->getCond(), Ctx);
+ }
+ checkBody<TerminationStmt>(BodyCS, Continuation);
+}
+} // namespace
+
+static bool fallbackBracesCheckActive(llvm::Optional<bool> Cur,
+ ClangTidyContext *Context) {
+ if (Cur)
+ return *Cur;
+ static constexpr size_t BSize = 64; // Should only ever need 63 chars here.
+ char Buff[BSize];
+ constexpr llvm::StringLiteral OptName = ".ShortStatementLines";
+ constexpr llvm::StringLiteral CheckAliases[] = {
+ "google-readability-braces-around-statements",
+ "readability-braces-around-statements", "hicpp-braces-around-statements"};
+ for (StringRef CheckAlias : CheckAliases) {
+ if (!Context->isCheckEnabled(CheckAlias))
+ continue;
+ assert(BSize >= CheckAlias.size() + OptName.size());
+ memcpy(Buff, CheckAlias.data(), CheckAlias.size());
+ memcpy(Buff + CheckAlias.size(), OptName.data(), OptName.size());
+ auto Iter = Context->getOptions().CheckOptions.find(
+ {Buff, CheckAlias.size() + OptName.size()});
+ if (Iter == Context->getOptions().CheckOptions.end())
+ return true;
+ unsigned Value;
+ if (StringRef(Iter->getValue().Value).getAsInteger(10, Value) || Value < 2)
+ return true;
+ }
+ return false;
+}
+
+UseEarlyExitsCheck::UseEarlyExitsCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ LineCountThreshold(Options.get("LineCountThreshold", 10U)),
+ SplitConjunctions(Options.get("SplitConjunctions", false)),
+ WrapInBraces(fallbackBracesCheckActive(Options.get<bool>("WrapInBraces"),
+ Context)) {}
+
+void UseEarlyExitsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "LineCountThreshold", LineCountThreshold);
+ Options.store(Opts, "SplitConjunctions", SplitConjunctions);
+ Options.store(Opts, "WrapInBraces", WrapInBraces);
+}
+
+void UseEarlyExitsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(translationUnitDecl(), this);
+}
+
+void UseEarlyExitsCheck::check(const MatchFinder::MatchResult &Result) {
+ EarlyExitVisitor(*this, *Result.Context).traverse();
+}
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -51,6 +51,7 @@
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
#include "UseAnyOfAllOfCheck.h"
+#include "UseEarlyExitsCheck.h"
namespace clang {
namespace tidy {
@@ -143,6 +144,8 @@
"readability-uppercase-literal-suffix");
CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
"readability-use-anyofallof");
+ CheckFactories.registerCheck<UseEarlyExitsCheck>(
+ "readability-use-early-exits");
}
};
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -48,6 +48,7 @@
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
UseAnyOfAllOfCheck.cpp
+ UseEarlyExitsCheck.cpp
LINK_LIBS
clangTidy
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
