[clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces' #162361

capitan-davide · 2025-10-07T20:04:47Z

github-actions · 2025-10-07T20:08:24Z

✅ With the latest revision this PR passed the C/C++ code formatter.

ojhunt · 2025-10-09T04:32:54Z

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedIf = Result.Nodes.getNodeAs<IfStmt>("if_stmt");
+
+  if (shouldHaveBraces(MatchedIf) && !doesHaveBraces(MatchedIf)) {


nit: llvm style is a single statement should not have a brace - hilariously violating this check (different code bases have differing policies so that's not a strike against this check :D)

I realize this is a draft but I saw it while going through open PRs and was wondering what you were diagnosing :)

github-actions · 2025-10-22T19:20:42Z

✅ With the latest revision this PR passed the C/C++ code linter.

llvmbot · 2025-10-25T07:08:54Z

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Davide Cunial (capitan-davide)

Changes

Closes #162140

Full diff: https://github.com/llvm/llvm-project/pull/162361.diff

9 Files Affected:

(modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
(modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
(added) clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp (+89)
(added) clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h (+41)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
(added) clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst (+42)
(modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+2-1)
(added) clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp (+55)
(added) clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp (+123)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index fe261e729539c..e1e42d22c520e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -33,6 +33,7 @@
 #include "ImplicitWideningOfMultiplicationResultCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncDecInConditionsCheck.h"
+#include "InconsistentIfelseBracesCheck.h"
 #include "IncorrectEnableIfCheck.h"
 #include "IncorrectEnableSharedFromThisCheck.h"
 #include "IncorrectRoundingsCheck.h"
@@ -150,6 +151,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-implicit-widening-of-multiplication-result");
     CheckFactories.registerCheck<InaccurateEraseCheck>(
         "bugprone-inaccurate-erase");
+    CheckFactories.registerCheck<InconsistentIfelseBracesCheck>(
+        "bugprone-inconsistent-ifelse-braces");
     CheckFactories.registerCheck<IncorrectEnableIfCheck>(
         "bugprone-incorrect-enable-if");
     CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 46bc8efd44bc5..d19fd5017d2e0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -28,6 +28,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   ForwardingReferenceOverloadCheck.cpp
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
+  InconsistentIfelseBracesCheck.cpp
   IncorrectEnableIfCheck.cpp
   IncorrectEnableSharedFromThisCheck.cpp
   InvalidEnumDefaultInitializationCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp
new file mode 100644
index 0000000000000..0c6b3e3e3ff9c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp
@@ -0,0 +1,89 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "InconsistentIfelseBracesCheck.h"
+#include "../utils/BracesAroundStatement.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+/// Check that at least one branch of the \p If statement is a \c CompoundStmt.
+static bool shouldHaveBraces(const IfStmt *If) {
+  const Stmt *const Then = If->getThen();
+  if (isa<CompoundStmt>(Then))
+    return true;
+
+  if (const Stmt *const Else = If->getElse()) {
+    if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+      return shouldHaveBraces(NestedIf);
+
+    return isa<CompoundStmt>(Else);
+  }
+
+  return false;
+}
+
+void InconsistentIfelseBracesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      traverse(TK_IgnoreUnlessSpelledInSource,
+               ifStmt(hasElse(anything()),
+                      unless(isConsteval()), // 'if consteval' always has braces
+                      unless(hasParent(ifStmt())))
+                   .bind("if_stmt")),
+      this);
+}
+
+void InconsistentIfelseBracesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedIf = Result.Nodes.getNodeAs<IfStmt>("if_stmt");
+  if (!shouldHaveBraces(MatchedIf))
+    return;
+  checkIfStmt(Result, MatchedIf);
+}
+
+void InconsistentIfelseBracesCheck::checkIfStmt(
+    const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If) {
+  const Stmt *Then = If->getThen();
+  if (const auto *NestedIf = dyn_cast<const IfStmt>(Then)) {
+    // If the then-branch is a nested IfStmt, first we need to add braces to
+    // it, then we need to check the inner IfStmt.
+    checkStmt(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+    if (shouldHaveBraces(NestedIf))
+      checkIfStmt(Result, NestedIf);
+  } else if (!isa<CompoundStmt>(Then)) {
+    checkStmt(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+  }
+
+  if (const Stmt *const Else = If->getElse()) {
+    if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+      checkIfStmt(Result, NestedIf);
+    else if (!isa<CompoundStmt>(Else))
+      checkStmt(Result, If->getElse(), If->getElseLoc());
+  }
+}
+
+void InconsistentIfelseBracesCheck::checkStmt(
+    const ast_matchers::MatchFinder::MatchResult &Result, const Stmt *S,
+    SourceLocation StartLoc, SourceLocation EndLocHint) {
+  const SourceManager &SM = *Result.SourceManager;
+  const LangOptions &LangOpts = Result.Context->getLangOpts();
+
+  const utils::BraceInsertionHints Hints =
+      utils::getBraceInsertionsHints(S, LangOpts, SM, StartLoc, EndLocHint);
+  if (Hints) {
+    DiagnosticBuilder Diag = diag(Hints.DiagnosticPos, "<message>");
+    if (Hints.offersFixIts()) {
+      Diag << Hints.openingBraceFixIt() << Hints.closingBraceFixIt();
+    }
+  }
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h
new file mode 100644
index 0000000000000..c818f46fea281
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h
@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects `if`/`else` statements where one branch uses braces and the other
+/// does not.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.html
+class InconsistentIfelseBracesCheck : public ClangTidyCheck {
+public:
+  InconsistentIfelseBracesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+private:
+  void checkIfStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                   const IfStmt *If);
+  void checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                 const Stmt *S, SourceLocation StartLoc,
+                 SourceLocation EndLocHint = SourceLocation());
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7cdff86beeec6..28bebceef1006 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -157,6 +157,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-inconsistent-ifelse-braces
+  <clang-tidy/checks/bugprone/inconsistent-ifelse-braces>` check.
+
+  Detects ``if``/``else`` statements where one branch uses braces and the other
+  does not.
+
 - New :doc:`bugprone-invalid-enum-default-initialization
   <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst
new file mode 100644
index 0000000000000..44f2d64452393
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst
@@ -0,0 +1,42 @@
+.. title:: clang-tidy - bugprone-inconsistent-ifelse-braces
+
+bugprone-inconsistent-ifelse-braces
+===================================
+
+Detects ``if``/``else`` statements where one branch uses braces and the other
+does not.
+
+Before:
+
+.. code-block:: c++
+
+  if (condition) {
+    statement;
+  } else
+    statement;
+
+  if (condition)
+    statement;
+  
+  if (condition)
+    statement;
+  else
+    statement;
+
+After:
+
+.. code-block:: c++
+
+  if (condition) {
+    statement;
+  } else {
+    statement;
+  }
+
+  if (condition)
+    statement;
+  
+  if (condition)
+    statement;
+  else
+    statement;
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c490d2ece2e0a..4c5b70f6c47e5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -101,6 +101,7 @@ Clang-Tidy Checks
    :doc:`bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result>`, "Yes"
    :doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
    :doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
+   :doc:`bugprone-inconsistent-ifelse-braces <bugprone/inconsistent-ifelse-braces>`, "Yes"
    :doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
    :doc:`bugprone-incorrect-enable-shared-from-this <bugprone/incorrect-enable-shared-from-this>`, "Yes"
    :doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
@@ -256,7 +257,7 @@ Clang-Tidy Checks
    :doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
    :doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
    :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
-   :doc:`llvm-use-ranges <llvm/use-ranges>`, "Yes"
+   :doc:`llvm-use-ranges <llvm/use-ranges>`,
    :doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
    :doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,
    :doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp
new file mode 100644
index 0000000000000..3eb51c39e2921
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy -std=c++23-or-later %s bugprone-inconsistent-ifelse-braces %t
+
+bool cond(const char *) { return false; }
+
+void do_something(const char *) {}
+
+// Positive tests.
+void f() {
+  if consteval {
+    if (cond("if1"))
+      do_something("if-single-line");
+    else {
+    }
+    // CHECK-MESSAGES: :[[@LINE-4]]:21: warning: <message> [bugprone-inconsistent-ifelse-braces]
+    // CHECK-FIXES: if (cond("if1")) {
+    // CHECK-FIXES: } else {
+  }
+
+  if consteval {
+    if (cond("if2"))
+      do_something("if-single-line");
+    else {
+    }
+    // CHECK-MESSAGES: :[[@LINE-4]]:21: warning: <message> [bugprone-inconsistent-ifelse-braces]
+    // CHECK-FIXES: if (cond("if2")) {
+    // CHECK-FIXES: } else {
+  } else {
+    if (cond("if2.1")) {
+    } else
+      do_something("else-single-line");
+    // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: <message> [bugprone-inconsistent-ifelse-braces]
+    // CHECK-FIXES: } else {
+    // CHECK-FIXES: }
+  }
+}
+
+// Negative tests.
+void g() {
+  if consteval {
+    if (cond("if0")) {
+      do_something("if-single-line");
+    } else if (cond("if0")) {
+      do_something("elseif-single-line");
+    } else {
+      do_something("else-single-line");
+    }
+  } else {
+    if (cond("if0.1"))
+      do_something("if-single-line");
+    else if (cond("if0.1"))
+      do_something("elseif-single-line");
+    else
+      do_something("else-single-line");
+  }
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp
new file mode 100644
index 0000000000000..3d4af258137ce
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-inconsistent-ifelse-braces %t
+
+bool cond(const char *) { return false; }
+
+void do_something(const char *) {}
+
+// Positive tests.
+void f() {
+  if (cond("if0") /*comment*/) do_something("if-same-line");
+  else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:31: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if0") /*comment*/) { do_something("if-same-line");
+  // CHECK-FIXES: } else {
+
+  if (cond("if0.1") /*comment*/) {
+  } else do_something("else-same-line");
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else { do_something("else-same-line");
+  // CHECK-FIXES: }
+
+  if (cond("if1"))
+    do_something("if-single-line");
+  else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if1")) {
+  // CHECK-FIXES: } else {
+
+  if (cond("if1.1")) {
+  } else
+    do_something("else-single-line");
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: }
+
+  if (cond("if2") /*comment*/)
+    // some comment
+    do_something("if-multi-line");
+  else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-5]]:31: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if2") /*comment*/) {
+  // CHECK-FIXES: } else {
+
+  if (cond("if2.1") /*comment*/) {
+  } else
+    // some comment
+    do_something("else-multi-line");
+  // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: }
+
+  if (cond("if3")) do_something("elseif-same-line");
+  else if (cond("if3")) {
+  } else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if3")) { do_something("elseif-same-line");
+  // CHECK-FIXES: } else if (cond("if3")) {
+
+  if (cond("if3.1")) {
+  } else if (cond("if3.1")) do_something("elseif-same-line");
+  else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:28: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else if (cond("if3.1")) { do_something("elseif-same-line");
+  // CHECK-FIXES: } else {
+
+  if (cond("if3.2")) {
+  } else if (cond("if3.2")) {
+  } else do_something("else-same-line");
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else { do_something("else-same-line");
+  // CHECK-FIXES: }
+
+  if (cond("if4-outer"))
+    if (cond("if4-inner"))
+      do_something("if-single-line");
+    else {
+    }
+  else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-7]]:25: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-MESSAGES: :[[@LINE-7]]:27: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if4-outer")) {
+  // CHECK-FIXES: if (cond("if4-inner")) {
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: } else {
+
+  if (cond("if5"))
+      do_something("if-single-line");
+  else if (cond("if5")) {
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (cond("if5")) {
+  // CHECK-FIXES: } else if (cond("if5")) {
+
+  if (cond("if5.1")) {
+  } else if (cond("if5.1"))
+      do_something("elseif-single-line");
+  // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: <message> [bugprone-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else if (cond("if5.1")) {
+  // CHECK-FIXES: }
+}
+
+// Negative tests.
+void g() {
+  if (cond("if0")) {
+    do_something("if-single-line");
+  } else if (cond("if0")) {
+    do_something("elseif-single-line");
+  } else {
+    do_something("else-single-line");
+  }
+
+  if (cond("if1"))
+    do_something("if-single-line");
+  else if (cond("if1"))
+    do_something("elseif-single-line");
+  else
+    do_something("else-single-line");
+}

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h

…races'

clang-tools-extra/docs/clang-tidy/checks/list.rst

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h

clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces.cpp

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp

Co-authored-by: Victor Chernyakin <[email protected]>

vbvictor

To me, this is purely readability check readability-inconsistent-ifelse-braces. I would leave bugprone category for checks that catch pattern which almost certainly have a bug (or very easy to have a bug). With such inconsistent braces, I don't think it's very easy to get a bug. We already have readability-misleading-indentation that catches cases with braces that are likely to have a bug.

localspook

Looks good, but we should agree on the warning message and category before merging (I'll be somewhat unhelpful and say that I don't have a strong opinion on whether it should be readability or bugprone)

localspook · 2025-11-04T10:28:35Z

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

+  const utils::BraceInsertionHints Hints =
+      utils::getBraceInsertionsHints(S, LangOpts, SM, StartLoc, EndLocHint);
+  if (Hints) {
+    DiagnosticBuilder Diag = diag(Hints.DiagnosticPos, "<message>");


Just leaving a comment to make sure this PR doesn't accidentally get merged without an actual message

capitan-davide · 2025-11-04T17:57:31Z

Also no strong opinions about module name. Thinking out loud, maybe this could be even made to implement LLVM Coding Standard exactly and made into an llvm-* check?

vbvictor · 2025-11-04T22:14:33Z

Also no strong opinions about module name. Thinking out loud, maybe this could be even made to implement LLVM Coding Standard exactly and made into an llvm-* check?

I was thinking about this check, but LLVM guide has so many caveats that I'm afraid it would be too strict and wouldn't satisfy the majority of users (Luckily, we have a whole codebase to test the check).
Instead of building "in one go", we can start from "building small" and first implement easy and time-proven things like "omit braces if the body consists of a single simple statement.". That would be very valuable IMO.

P.S. clang-format already implement part of this functionality in https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm.

In total, It's up to you what you want to be implemented, we appreciate all contributions. Your check is valuable in current form, and we can reuse some written functions/matchers when implementing "LLVM" check.

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

vbvictor · 2025-11-04T22:31:46Z

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

+  const utils::BraceInsertionHints Hints =
+      utils::getBraceInsertionsHints(S, LangOpts, SM, StartLoc, EndLocHint);
+  if (Hints) {
+    DiagnosticBuilder Diag = diag(Hints.DiagnosticPos, "<message>");


Why are we only emitting diagnostics if we can insert hints?
Can there be a situation if we can't emit hint, but still need to emit diagnostics for user to manually fix.

Why are we only emitting diagnostics if we can insert hints? Can there be a situation if we can't emit hint, but still need to emit diagnostics for user to manually fix.

I updated this to assert that FitIts are always available, when the source location is not a macro expansion. It runs on the clang+clang-tools-extra codebase with 6013 findings and no crashes.

In case the location is a macro expansion we can either:

warn, but no fix-its

ignore it (like readability-braces-around-statements)

vbvictor · 2025-11-04T22:32:19Z

clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp

@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-inconsistent-ifelse-braces %t


Please add tests with macros

vbvictor · 2025-11-04T22:33:38Z

clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp

+void InconsistentIfelseBracesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedIf = Result.Nodes.getNodeAs<IfStmt>("if_stmt");
+  if (!shouldHaveBraces(MatchedIf))


Instead of matching each if statement, we should be able to check shouldHaveBraces inside addMatcher.
Create custom AST-matcher on IfStmt and run shouldHaveBraces on it.

Uhm, I am not familiar with other matchers that tries to enforce some kind of "policy". What would be the benefits of this approach? IMHO it hurts readability a bit.

capitan-davide · 2025-11-08T07:37:28Z

To me, this is purely readability check readability-inconsistent-ifelse-braces. I would leave bugprone category for checks that catch pattern which almost certainly have a bug (or very easy to have a bug). With such inconsistent braces, I don't think it's very easy to get a bug. We already have readability-misleading-indentation that catches cases with braces that are likely to have a bug.

Renamed to readability-*

Also, renamed from `bugprone-*` to `readability-*`

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch from 693bcf3 to 29e7a6a Compare October 8, 2025 18:24

ojhunt reviewed Oct 9, 2025

View reviewed changes

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch 2 times, most recently from b35337a to 542f435 Compare October 22, 2025 19:15

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch 2 times, most recently from 692b19f to a0a7126 Compare October 24, 2025 06:17

capitan-davide added 2 commits October 24, 2025 08:18

[clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces'

5f61932

[clang-tidy] Add some tests to 'bugprone-inconsistent-ifelse-braces'

18e6ae8

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch 2 times, most recently from e54909d to 28a213b Compare October 25, 2025 06:43

capitan-davide marked this pull request as ready for review October 25, 2025 07:08

llvmbot added clang-tools-extra clang-tidy labels Oct 25, 2025

capitan-davide mentioned this pull request Oct 25, 2025

[clang-tidy] New check: bugprone-ifelse-braces #162140

Open

zwuis reviewed Oct 27, 2025

View reviewed changes

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h Outdated Show resolved Hide resolved

capitan-davide added 2 commits October 27, 2025 19:09

[clang-tidy] Update documentation for 'bugprone-inconsistent-ifelse-b…

117eb01

…races'

[clang-tidy] Apply suggestions from code review

40570f9

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch from 28a213b to 40570f9 Compare October 27, 2025 18:11

capitan-davide requested a review from ojhunt October 27, 2025 18:13

localspook reviewed Oct 27, 2025

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp Outdated Show resolved Hide resolved

capitan-davide and others added 2 commits October 28, 2025 13:11

[clang-tidy] Apply suggestions from code review

6a652cd

Co-authored-by: Victor Chernyakin <[email protected]>

[clang-tidy] Add some test for constexpr if

540a43c

vbvictor reviewed Nov 2, 2025

View reviewed changes

localspook approved these changes Nov 4, 2025

View reviewed changes

vbvictor reviewed Nov 4, 2025

View reviewed changes

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch 3 times, most recently from 7e1efdd to b660692 Compare November 8, 2025 07:36

[clang-tidy] Add some suggestions from code review

02aa926

Also, renamed from `bugprone-*` to `readability-*`

capitan-davide force-pushed the bugprone-inconsistent-ifelse-braces branch from b660692 to 02aa926 Compare November 8, 2025 07:55

		@@ -0,0 +1,122 @@
		// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-inconsistent-ifelse-braces %t

[clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces' #162361

Are you sure you want to change the base?

[clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces' #162361

Uh oh!

Conversation

capitan-davide commented Oct 7, 2025

Uh oh!

github-actions bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ojhunt Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vbvictor left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

localspook left a comment

Choose a reason for hiding this comment

Uh oh!

localspook Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

capitan-davide commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbvictor commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vbvictor Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

capitan-davide Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

vbvictor Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

vbvictor Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

capitan-davide Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capitan-davide commented Nov 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Oct 7, 2025 •

edited

Loading

github-actions bot commented Oct 22, 2025 •

edited

Loading

llvmbot commented Oct 25, 2025 •

edited

Loading

vbvictor left a comment •

edited

Loading

capitan-davide commented Nov 4, 2025 •

edited

Loading

vbvictor commented Nov 4, 2025 •

edited

Loading

capitan-davide Nov 5, 2025 •

edited

Loading