[clang-tools-extra] [clang-tidy] Ignore label-like statements when checking if/else bodies (PR #202869)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Gaurav Dhingra (gxyd)

<details>
<summary>Changes</summary>

Fixes #<!-- -->194694

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp (+4-11) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp (+126) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
index fc22e36a67c54..732b5f199230f 100644
--- a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
@@ -18,21 +18,14 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::readability {
 
-/// Look through AttributedStmt wrappers to find the underlying statement.
-static const Stmt *ignoreAttributed(const Stmt *S) {
-  if (const auto *AS = dyn_cast<AttributedStmt>(S))
-    return AS->getSubStmt();
-  return S;
-}
-
 /// 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 = ignoreAttributed(If->getThen());
+  const Stmt *const Then = If->getThen()->stripLabelLikeStatements();
   if (isa<CompoundStmt>(Then))
     return true;
 
   if (const Stmt *Else = If->getElse()) {
-    Else = ignoreAttributed(Else);
+    Else = Else->stripLabelLikeStatements();
     if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
       return shouldHaveBraces(NestedIf);
 
@@ -61,7 +54,7 @@ void InconsistentIfElseBracesCheck::check(
 
 void InconsistentIfElseBracesCheck::checkIfStmt(
     const MatchFinder::MatchResult &Result, const IfStmt *If) {
-  const Stmt *Then = ignoreAttributed(If->getThen());
+  const Stmt *Then = If->getThen()->stripLabelLikeStatements();
   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.
@@ -74,7 +67,7 @@ void InconsistentIfElseBracesCheck::checkIfStmt(
   }
 
   if (const Stmt *Else = If->getElse()) {
-    Else = ignoreAttributed(Else);
+    Else = Else->stripLabelLikeStatements();
     if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
       checkIfStmt(Result, NestedIf);
     else if (!isa<CompoundStmt>(Else))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c369b1fd8b373..003b622e6821a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -780,6 +780,10 @@ Changes in existing checks
   - Fixed a false positive where ``bool`` conditions in C conditional
     operators were diagnosed as implicit conversions to ``int``.
 
+- Improved :doc:`readability-inconsistent-ifelse-braces
+  <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check to
+  correctly handle labeled statements of ``if``/``else`` bodies.
+
 - Improved :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability/non-const-parameter>` check:
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp
new file mode 100644
index 0000000000000..fcf9b1112a122
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp
@@ -0,0 +1,126 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s readability-inconsistent-ifelse-braces %t
+
+// Positive tests.
+void f(bool b) {
+  if (b) goo:
+    return;
+  else too: {
+    return;
+  }
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (b) { goo:
+  // CHECK-FIXES-NEXT:     return;
+  // CHECK-FIXES-NEXT: } else too: {
+
+  if (b) xoo: {
+    return;
+  } else yoo:
+    return;
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else { yoo:
+  // CHECK-FIXES-NEXT:   return;
+  // CHECK-FIXES-NEXT: }
+
+  if (b) hoo: {
+    return;
+  } else loo: [[unlikely]]
+    return;
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else { loo: {{[[][[]}}unlikely{{[]][]]}}
+  // CHECK-FIXES-NEXT:   return;
+  // CHECK-FIXES-NEXT: }
+
+  if (b)
+    return;
+  else coo: [[unlikely]] {
+    return;
+  }
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (b) {
+  // CHECK-FIXES-NEXT:   return;
+  // CHECK-FIXES-NEXT: } else coo: {{[[][[]}}unlikely{{[]][]]}} {
+
+  if (b) aoo:
+    return;
+  else boo: [[unlikely]] {
+    return;
+  }
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (b) { aoo:
+  // CHECK-FIXES-NEXT:   return;
+  // CHECK-FIXES-NEXT: } else boo: {{[[][[]}}unlikely{{[]][]]}} {
+
+  if (b) moo: [[unlikely]]
+    return;
+  else noo: [[unlikely]] {
+    return;
+  }
+  // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: if (b) { moo: {{[[][[]}}unlikely{{[]][]]}}
+  // CHECK-FIXES-NEXT:   return;
+  // CHECK-FIXES-NEXT: } else noo: {{[[][[]}}unlikely{{[]][]]}} {
+
+  if (b) poo: [[likely]] {
+    return;
+  } else qoo: [[unlikely]]
+    return;
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+  // CHECK-FIXES: } else { qoo: {{[[][[]}}unlikely{{[]][]]}}
+  // CHECK-FIXES-NEXT:    return;
+  // CHECK-FIXES-NEXT: }
+}
+
+
+// Negative tests.
+void g(bool b) {
+  if (b) {
+    return;
+  } else foo: {
+    return;
+  }
+
+  if (b) {
+    return;
+  } else goo: [[unlikely]] {
+    return;
+  }
+
+  if (b) {
+    return;
+  } else hoo: {
+    return;
+  }
+
+  if (b) joo: {
+    return;
+  } else {
+    return;
+  }
+
+  if (b) roo: [[unlikely]] {
+    return;
+  } else {
+    return;
+  }
+
+  if (b) koo: {
+    return;
+  } else loo: {
+    return;
+  }
+
+  if (b) moo:
+    return;
+  else noo:
+    return;
+
+  if (b)
+    return;
+  else ooo:
+    return;
+
+  if (b) poo: [[likely]]
+    return;
+  else qoo: [[unlikely]]
+    return;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/202869