njames93 updated this revision to Diff 448172.
njames93 added a comment.
Fix typo in test file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130630/new/
https://reviews.llvm.org/D130630
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/NestedIfsCheck.cpp
clang-tools-extra/clang-tidy/readability/NestedIfsCheck.h
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability/nested-ifs.rst
clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s readability-nested-ifs %t
+
+void foo();
+void bar();
+
+bool cond(int X = 0);
+
+void good() {
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+ if (cond()) {
+ if (cond(1)) {
+ foo();
+ bar();
+ }
+ } // End
+ // CHECK-FIXES: if (cond() && cond(1))
+ // CHECK-FIXES-NEXT: {
+ // CHECK-FIXES-NEXT: foo();
+ // CHECK-FIXES-NEXT: bar();
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: // End
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Nested ifs can be combined
+ // CHECK-MESSAGES: :[[@LINE+4]]:7: warning: Nested ifs can be combined
+ if (cond()) {
+ if (cond(1)) {
+ foo();
+ if (cond(2)) {
+ if (cond(3)) {
+ bar();
+ }
+ }
+ }
+ } // End
+
+ // CHECK-FIXES: if (cond() && cond(1))
+ // CHECK-FIXES-NEXT: {
+ // CHECK-FIXES-NEXT: foo();
+ // CHECK-FIXES-NEXT: if (cond(2) && cond(3))
+ // CHECK-FIXES-NEXT: {
+ // CHECK-FIXES-NEXT: bar();
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: {{ }}
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: // End
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:5: warning: Nested ifs can be combined
+ if (bool B = cond()) {
+ if (cond(1)) {
+ if (cond(2)) {
+ foo();
+ }
+ }
+ } // End
+
+ // CHECK-FIXES: if (bool B = cond()) {
+ // CHECK-FIXES-NEXT: if (cond(1) && cond(2))
+ // CHECK-FIXES-NEXT: {
+ // CHECK-FIXES-NEXT: foo();
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: {{ }}
+ // CHECK-FIXES-NEXT: } // End
+}
+
+void bad() {
+ // Condition variable on either nesting can't be converted.
+ if (bool B = cond())
+ if (cond())
+ foo();
+ if (cond()) {
+ if (bool B = cond())
+ bar();
+ }
+
+ // Can only have a single if statement as the body of the outer if.
+ if (cond()) {
+ foo();
+ if (cond())
+ bar();
+ }
+ if (cond()) {
+ if (cond())
+ foo();
+ bar();
+ }
+
+ // Can't have an else statement on either if.
+ if (cond()) {
+ if (cond())
+ foo();
+ else
+ bar();
+ }
+
+ if (cond()) {
+ if (cond())
+ foo();
+ } else {
+ bar();
+ }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s readability-nested-ifs %t -- -- -fno-delayed-template-parsing
+
+// Ensure the check handles if initializers and constexpr if statements
+// correctly.
+
+void foo();
+void bar();
+
+bool cond(int X = 0);
+
+void good() {
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+ if (bool X = cond(); X) {
+ if (cond()) {
+ if (cond(1))
+ bar();
+ }
+ } // End
+ // CHECK-FIXES: if (bool X = cond(); X && cond() && cond(1))
+ // CHECK-FIXES-NEXT: {{ }}
+ // CHECK-FIXES-NEXT: {{ }}
+ // CHECK-FIXES-NEXT: bar();
+ // CHECK-FIXES-NEXT: {{ }}
+ // CHECK-FIXES-NEXT: // End
+
+ // The initializer doesn't have to be a variable declaration.
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+ if (foo(); true) {
+ if (cond())
+ bar();
+ }
+}
+
+void bad() {
+ // Only the outermost `if` statement can have an initializer.
+ if (bool X = cond(); X) {
+ if (bool Y = cond(1); Y) {
+ bar();
+ }
+ }
+
+ if (cond()) {
+ if (bool X = cond(1); X) {
+ bar();
+ }
+ }
+}
+
+template <bool B, bool C> void constexprIfs() {
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+ if constexpr (B) {
+ if constexpr (C) {
+ bar();
+ }
+ } // End
+ // CHECK-FIXES: if constexpr (B && C)
+ // CHECK-FIXES-NEXT: {
+ // CHECK-FIXES-NEXT: bar();
+ // CHECK-FIXES-NEXT: }
+ // CHECK-FIXES-NEXT: // End
+
+ // constexpr ifs are only converted if both ifs are constexpr.
+ if constexpr (B) {
+ if (C) {
+ bar();
+ }
+ }
+
+ if (B) {
+ if constexpr (C) {
+ bar();
+ }
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/nested-ifs.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/nested-ifs.rst
@@ -0,0 +1,57 @@
+.. title:: clang-tidy - readability-nested-ifs
+
+readability-nested-ifs
+======================
+
+Detects nested ``if`` statements that could be merged into one by ANDing the
+conditions.
+
+This transformation requires that the outer ``if`` contains a single ``if`` as
+its then branch. Both ``if`` statements cannot have an ``else`` branch,
+condition variable or have a ``consteval`` specifier.
+
+Example:
+
+.. code-block:: c++
+
+ void foo() {
+ if (Condition1) {
+ if (Condition2) {
+ Process();
+ }
+ }
+ }
+
+Would be transformed to:
+
+.. code-block:: c++
+
+ void foo() {
+ if (Condition1 && Condition2) {
+ Process();
+ }
+ }
+
+This will also recurse to enable merging `N` nested ifs.
+
+.. code-block:: c++
+
+ void foo() {
+ if (Condition1) {
+ if (Condition...) {
+ if (ConditionN) {
+ Process()
+ }
+ }
+ }
+ }
+
+Would be transformed to:
+
+.. code-block:: c++
+
+ void foo() {
+ if (Condition1 && Condition... && ConditionN) {
+ Process()
+ }
+ }
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
@@ -340,6 +340,7 @@
`readability-misleading-indentation <readability/misleading-indentation.html>`_,
`readability-misplaced-array-index <readability/misplaced-array-index.html>`_, "Yes"
`readability-named-parameter <readability/named-parameter.html>`_, "Yes"
+ `readability-nested-ifs <readability/nested-ifs.html>`_, "Yes"
`readability-non-const-parameter <readability/non-const-parameter.html>`_, "Yes"
`readability-qualified-auto <readability/qualified-auto.html>`_, "Yes"
`readability-redundant-access-specifiers <readability/redundant-access-specifiers.html>`_, "Yes"
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-nested-ifs
+ <clang-tidy/checks/readability/nested-ifs>` check.
+
+ Detects nested ``if`` statements that could be merged into one by ANDing the
+ conditions.
+
New check aliases
^^^^^^^^^^^^^^^^^
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
@@ -31,6 +31,7 @@
#include "MisleadingIndentationCheck.h"
#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
+#include "NestedIfsCheck.h"
#include "NonConstParameterCheck.h"
#include "QualifiedAutoCheck.h"
#include "RedundantAccessSpecifiersCheck.h"
@@ -101,6 +102,7 @@
"readability-misleading-indentation");
CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
"readability-misplaced-array-index");
+ CheckFactories.registerCheck<NestedIfsCheck>("readability-nested-ifs");
CheckFactories.registerCheck<QualifiedAutoCheck>(
"readability-qualified-auto");
CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
Index: clang-tools-extra/clang-tidy/readability/NestedIfsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/NestedIfsCheck.h
@@ -0,0 +1,35 @@
+//===--- NestedIfsCheck.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_NESTEDIFSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NESTEDIFSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detects nested `if` statements that could be merged into one by ANDing the
+/// conditions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/nested-ifs.html
+class NestedIfsCheck : public ClangTidyCheck {
+public:
+ NestedIfsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NESTEDIFSCHECK_H
Index: clang-tools-extra/clang-tidy/readability/NestedIfsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/NestedIfsCheck.cpp
@@ -0,0 +1,116 @@
+//===--- NestedIfsCheck.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 "NestedIfsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+class NestedIfVisitor : public RecursiveASTVisitor<NestedIfVisitor> {
+ using Base = RecursiveASTVisitor<NestedIfVisitor>;
+ NestedIfsCheck &Check;
+ ASTContext &Ctx;
+
+public:
+ NestedIfVisitor(NestedIfsCheck &Check, ASTContext &Ctx)
+ : Check(Check), Ctx(Ctx) {}
+
+ void traverse() { TraverseAST(Ctx); }
+
+ static bool isCanditateIf(const IfStmt *If, bool IsFirstConstexpr,
+ bool Outer = false) {
+ if (If->isConsteval())
+ return false;
+ if (!Outer && IsFirstConstexpr != If->isConstexpr())
+ return false;
+ if (If->hasElseStorage() || If->hasVarStorage())
+ return false;
+ if (!If->getThen())
+ return false;
+ // The first `if` in the chain can have an init statement, but any nested
+ // ones cannot.
+ return Outer || !If->hasInitStorage();
+ }
+
+ bool captureNested(IfStmt *If, SmallVectorImpl<const IfStmt *> &Capture,
+ DataRecursionQueue *Queue) {
+ auto AllowConstexpr = If->isConstexpr();
+ while (true) {
+ auto *Nested = dyn_cast<IfStmt>(If->getThen());
+ if (!Nested) {
+ if (auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
+ if (CS->size() == 1)
+ Nested = dyn_cast<IfStmt>(CS->body_front());
+ }
+ }
+ if (!Nested || !isCanditateIf(Nested, AllowConstexpr))
+ return Base::TraverseIfStmt(If, Queue);
+
+ Capture.push_back(Nested);
+ If = Nested;
+ }
+ return true;
+ }
+
+ bool TraverseIfStmt(IfStmt *If, DataRecursionQueue *Queue = nullptr) {
+ if (!isCanditateIf(If, false, true))
+ return Base::TraverseIfStmt(If, Queue);
+ SmallVector<const IfStmt *, 3> Diagnose{If};
+ auto Result = captureNested(If, Diagnose, Queue);
+ if (Diagnose.size() == 1)
+ return Result;
+ auto D = Check.diag(If->getBeginLoc(), "Nested ifs can be combined");
+ for (const IfStmt *Item : Diagnose)
+ D << SourceRange(Item->getBeginLoc(), Item->getRParenLoc());
+ SmallString<128> CondAppend;
+ for (auto P = Diagnose.begin(), C = std::next(P), E = Diagnose.end();
+ C != E; P = C++) {
+ const IfStmt *Prev = *P;
+ if (const auto *CS = dyn_cast<CompoundStmt>(Prev->getThen()))
+ D << FixItHint::CreateRemoval(CS->getLBracLoc())
+ << FixItHint::CreateRemoval(CS->getRBracLoc());
+ const IfStmt *Cur = *C;
+ CondAppend.append(
+ {" && ",
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Cur->getCond()->getSourceRange()),
+ Ctx.getSourceManager(), Ctx.getLangOpts())});
+ D << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ {Cur->getBeginLoc(), Cur->getRParenLoc()}));
+ }
+ D << FixItHint::CreateInsertion(If->getRParenLoc(), CondAppend);
+
+ return Result;
+ }
+};
+} // namespace
+
+void NestedIfsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(translationUnitDecl(), this);
+}
+
+void NestedIfsCheck::check(const MatchFinder::MatchResult &Result) {
+ NestedIfVisitor(*this, *Result.Context).traverse();
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
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
@@ -27,6 +27,7 @@
MisplacedArrayIndexCheck.cpp
NamedParameterCheck.cpp
NamespaceCommentCheck.cpp
+ NestedIfsCheck.cpp
NonConstParameterCheck.cpp
QualifiedAutoCheck.cpp
ReadabilityTidyModule.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
