[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel created this revision.
rymiel added reviewers: njames93, LegalizeAdulthood, aaron.ballman, gribozavr2.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The `readability-braces-around-statement` check tries to look at the
closing parens of the if condition to determine where to insert braces,
however, "consteval if" statements don't have a condition, and always
have braces regardless, so the skip can be checked.

The `readability-simplify-boolean-expr` check looks at the condition
of the if statement to determine what could be simplified, but as
"consteval if" statements do not have a condition that could be
simplified, they can also be skipped here.

There may still be more checks that try to look at the conditions of
`if`s that aren't included here

Fixes https://github.com/llvm/llvm-project/issues/57568


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133413

Files:
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp
@@ -0,0 +1,13 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++2b | count 0
+template <bool Cond>
+constexpr int foo() {
+  if consteval {
+    if constexpr (Cond) {
+      return 0;
+    } else {
+      return 1;
+    }
+  } else {
+    return 2;
+  }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
@@ -0,0 +1,31 @@
+// RUN: clang-tidy %s -checks='-*,readability-braces-around-statements' -- -std=c++2b | count 0
+
+constexpr void handle(bool) {}
+
+constexpr void shouldPass() {
+  if consteval {
+    handle(true);
+  } else {
+    handle(false);
+  }
+}
+
+constexpr void shouldPassNegated() {
+  if !consteval {
+    handle(false);
+  } else {
+    handle(true);
+  }
+}
+
+constexpr void shouldPassSimple() {
+  if consteval {
+    handle(true);
+  }
+}
+
+void run() {
+    shouldPass();
+    shouldPassNegated();
+    shouldPassSimple();
+}
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -354,8 +354,9 @@
   }
 
   bool VisitIfStmt(IfStmt *If) {
-    // Skip any if's that have a condition var or an init statement.
-    if (If->hasInitStorage() || If->hasVarStorage())
+    // Skip any if's that have a condition var or an init statement, or are
+    // "if consteval" statements.
+    if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())
       return true;
     /*
      * if (true) ThenStmt(); -> ThenStmt();
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -131,6 +131,10 @@
       return;
     checkStmt(Result, S->getBody(), StartLoc);
   } else if (const auto *S = Result.Nodes.getNodeAs<IfStmt>("if")) {
+    // "if consteval" always has braces.
+    if (S->isConsteval())
+      return;
+
     SourceLocation StartLoc = findRParenLoc(S, SM, Context);
     if (StartLoc.isInvalid())
       return;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133413.458421.patch
Type: text/x-patch
Size: 2776 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220907/2e8ce770/attachment-0001.bin>