[clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return.

[njames93]

Summary

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

[LegalizeAdulthood]

Another good catch!

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

private:
   void replaceWithReturnCondition(const ASTContext &Context, const IfStmt *If,
                                   const Expr *BoolLiteral, bool Negated);
 
   void replaceWithAssignment(const ASTContext &Context, const IfStmt *If,
                              const Expr *Var, SourceLocation Loc, bool Negated);
 
   void replaceCompoundReturnWithCondition(const ASTContext &Context,
                                           const ReturnStmt *Ret, bool Negated,
-                                          const IfStmt *If);
+                                          const IfStmt *If,
+                                          const Expr *ThenReturn);
 
   void issueDiag(const ASTContext &Result, SourceLocation Loc,
                  StringRef Description, SourceRange ReplacementRange,
                  StringRef Replacement);
 
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H

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

if (needsNullPtrComparison(E))
     return compareExpressionToNullPtr(Context, E, true);
 
   if (needsZeroComparison(E))
     return compareExpressionToZero(Context, E, true);
 
   return asBool(getText(Context, *E), NeedsStaticCast);
 }
 
-static const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
-  if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) {
-    if (Bool->getValue() == !Negated)
-      return Bool;
-  }
-  if (const auto *Unary = dyn_cast<UnaryOperator>(Ret->getRetValue())) {
-    if (Unary->getOpcode() == UO_LNot) {
-      if (const auto *Bool =
-              dyn_cast<CXXBoolLiteralExpr>(Unary->getSubExpr())) {
-        if (Bool->getValue() == Negated)
-          return Bool;
-      }
-    }
-  }
-
-  return nullptr;
-}
-
-static const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
-  if (IfRet->getElse() != nullptr)
-    return nullptr;
-
-  if (const auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen()))
-    return stmtReturnsBool(Ret, Negated);
-
-  if (const auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) {
-    if (Compound->size() == 1) {
-      if (const auto *CompoundRet = dyn_cast<ReturnStmt>(Compound->body_back()))
-        return stmtReturnsBool(CompoundRet, Negated);
-    }
-  }
-
-  return nullptr;
-}
-
 static bool containsDiscardedTokens(const ASTContext &Context,
                                     CharSourceRange CharRange) {
   std::string ReplacementText =
       Lexer::getSourceText(CharRange, Context.getSourceManager(),
                            Context.getLangOpts())
           .str();
   Lexer Lex(CharRange.getBegin(), Context.getLangOpts(), ReplacementText.data(),
             ReplacementText.data(),
for (auto First = CS->body_begin(), Second = std::next(First),
          */
         auto *If = cast<IfStmt>(*First);
         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);
+                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()) {
           ExprAndBool ThenReturnBool =
               checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
           if (ThenReturnBool &&
               ThenReturnBool.Bool != TrailingReturnBool.Bool) {
             Check->replaceCompoundReturnWithCondition(
                 Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
-                SubIf);
+                SubIf, ThenReturnBool.Item);
           }
         }
       }
     }
     return true;
   }
 
 private:
void SimplifyBooleanExprCheck::replaceWithReturnCondition(
   std::string Replacement = ("return " + Condition + Terminator).str();
   SourceLocation Start = BoolLiteral->getBeginLoc();
   issueDiag(Context, Start, SimplifyConditionalReturnDiagnostic,
             If->getSourceRange(), Replacement);
 }
 
 void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
     const ASTContext &Context, const ReturnStmt *Ret, bool Negated,
-    const IfStmt *If) {
-  const auto *Lit = stmtReturnsBool(If, Negated);
+    const IfStmt *If, const Expr *ThenReturn) {
   const std::string Replacement =
       "return " + replacementExpression(Context, Negated, If->getCond());
-  issueDiag(Context, Lit->getBeginLoc(), SimplifyConditionalReturnDiagnostic,
+  issueDiag(Context, ThenReturn->getBeginLoc(),
+            SimplifyConditionalReturnDiagnostic,
             SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
 }
 
 void SimplifyBooleanExprCheck::replaceWithAssignment(const ASTContext &Context,
                                                      const IfStmt *IfAssign,
                                                      const Expr *Var,
                                                      SourceLocation Loc,
                                                      bool Negated) {

clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

 #include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
+#include "readability/SimplifyBooleanExprCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
 using readability::BracesAroundStatementsCheck;
 using readability::NamespaceCommentCheck;
+using readability::SimplifyBooleanExprCheck;
 using namespace ast_matchers;
 
 // Copied from ASTMatchersTests
 static std::vector<TestClangConfig> allTestClangConfigs() {
   std::vector<TestClangConfig> all_configs;
   for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
                             Lang_CXX14, Lang_CXX17, Lang_CXX20}) {
     TestClangConfig config;
TEST(BracesAroundStatementsCheckTest, ImplicitCastInReturn) {
   EXPECT_EQ("const char *f() {\n"
             "  if (true) { return \"\";\n"
             "}\n"
             "  return \"abc\";\n"
             "}\n",
             runCheckOnCode<BracesAroundStatementsCheck>(Input));
 }
 
+TEST(SimplifyBooleanExprCheckTest, CodeWithError) {
+  // Fixes PR55557
+  // Need to downgrade Wreturn-type from error as runCheckOnCode will fatal_exit
+  // if any errors occur.
+  EXPECT_EQ("void foo(bool b){ return b; }",
+            runCheckOnCode<SimplifyBooleanExprCheck>(
+                "void foo(bool b){ if (b) return true; return false; }",
+                nullptr, "input.cc", {"-Wno-error=return-type"}));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang