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

[rymiel]

Summary

The readability-braces-around-statements 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
ifs that aren't included here

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

[rymiel]

Note for reviewing:

• The C++ standard, from what I could tell, seems to imply the the else clause of a consteval if clause could be written without braces, however it seems that both GCC and Clang don't allow for this, so both clauses of constexpr if statements should be skipped in readability-braces-around-statement
• I was really unsure what to do with the tests. Since consteval if is a C++23 feature, I felt like they needed to be in their own files, but these files are also super minimal, as they really only make sure that using them doesn't crash.
• The tests use the flag -std=c++2b. Seeing as that flag will probably be renamed once 2023 actually rolls around, is that okay?

[rymiel]

Ping? This is fixing a segmentation fault. Please let me know if there are other people I should add as reviewers

[aaron.ballman]

LGTM, though please add a release note for the fix. Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

[rymiel]

Add release note

Thank you, Aaron! Is the note I've added okay? I didn't expect to need to add a note to a bugfix, but I haven't written a release note before at all regardless. And yes, I'm able to commit it myself.

[njames93]

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

471–472

Probably should add a check here.

495

And here.

[rymiel]

Thank you for pointing those out, I was indeed able to cause crashes from those locations

[rymiel]

Extra consteval checks in SimplifyBooleanExprCheck

[njames93]

LGTM

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

void BracesAroundStatementsCheck::check(
   } else if (const auto *S = Result.Nodes.getNodeAs<DoStmt>("do")) {
     checkStmt(Result, S->getBody(), S->getDoLoc(), S->getWhileLoc());
   } else if (const auto *S = Result.Nodes.getNodeAs<WhileStmt>("while")) {
     SourceLocation StartLoc = findRParenLoc(S, SM, Context);
     if (StartLoc.isInvalid())
       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;
     if (ForceBracesStmts.erase(S))
       ForceBracesStmts.insert(S->getThen());
     bool BracedIf = checkStmt(Result, S->getThen(), StartLoc, S->getElseLoc());
     const Stmt *Else = S->getElse();
     if (Else && BracedIf)

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

static auto checkSingleStatement(Stmt *S, Functor F) -> decltype(F(S)) {
     return F(S);
   }
 
   Stmt *parent() const {
     return StmtStack.size() < 2 ? nullptr : StmtStack[StmtStack.size() - 2];
   }
 
   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();
      * if (false) ThenStmt(); -> <Empty>;
      * if (false) ThenStmt(); else ElseStmt() -> ElseStmt();
      */
     Expr *Cond = If->getCond()->IgnoreImplicit();
     if (Optional<bool> Bool = getAsBoolLiteral(Cond, true)) {
for (auto First = CS->body_begin(), Second = std::next(First),
         continue;
 
       if (CurIf) {
         /*
          * if (Cond) return true; return false; -> return Cond;
          * if (Cond) return false; return true; -> return !Cond;
          */
         auto *If = cast<IfStmt>(*First);
-        if (!If->hasInitStorage() && !If->hasVarStorage()) {
+        if (!If->hasInitStorage() && !If->hasVarStorage() &&
+            !If->isConsteval()) {
           ExprAndBool ThenReturnBool =
               checkSingleStatement(If->getThen(), parseReturnLiteralBool);
           if (ThenReturnBool &&
               ThenReturnBool.Bool != TrailingReturnBool.Bool) {
             if ((Check->ChainedConditionalReturn || !PrevIf) &&
                 If->getElse() == nullptr) {
               Check->replaceCompoundReturnWithCondition(
                   Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
                   If, ThenReturnBool.Item);
             }
           }
         }
       } else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {
         /*
          * (case X|label_X|default): if (Cond) return BoolLiteral;
          *                           return !BoolLiteral
          */
         Stmt *SubStmt =
             isa<LabelStmt>(*First)  ? cast<LabelStmt>(*First)->getSubStmt()
             : isa<CaseStmt>(*First) ? cast<CaseStmt>(*First)->getSubStmt()
                                     : cast<DefaultStmt>(*First)->getSubStmt();
         auto *SubIf = dyn_cast<IfStmt>(SubStmt);
         if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
-            !SubIf->hasVarStorage()) {
+            !SubIf->hasVarStorage() && !SubIf->isConsteval()) {
           ExprAndBool ThenReturnBool =
               checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
           if (ThenReturnBool &&
               ThenReturnBool.Bool != TrailingReturnBool.Bool) {
             Check->replaceCompoundReturnWithCondition(
                 Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
                 SubIf, ThenReturnBool.Item);
           }

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default>`
   with empty body is not equivalent to the explicitly defaulted one, variadic constructors
   since they cannot be explicitly defaulted. The check also skips copy assignment operators
   with nonstandard return types. The check is restricted to c++11-or-later.
 
 - Change the default behavior of :doc:`readability-avoid-const-params-in-decls
   <clang-tidy/checks/readability/avoid-const-params-in-decls>` to not
   warn about `const` value parameters of declarations inside macros.
 
+- Fixed crashes in :doc:`readability-braces-around-statements
+  <clang-tidy/checks/readability/braces-around-statements>` and
+  :doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>`
+  when using a C++23 ``if consteval`` statement.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp

+// 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();
+}

clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp

+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++2b | count 0
+template <bool Cond>
+constexpr int testIf() {
+  if consteval {
+    if constexpr (Cond) {
+      return 0;
+    } else {
+      return 1;
+    }
+  } else {
+    return 2;
+  }
+}
+
+constexpr bool testCompound() {
+  if consteval {
+    return true;
+  }
+  return false;
+}
+
+constexpr bool testCase(int I) {
+  switch (I) {
+    case 0: {
+      if consteval {
+        return true;
+      }
+      return false;
+    }
+    default: {
+      if consteval {
+        return false;
+      }
+      return true;
+    }
+  }
+}