Author: Nathan James Date: 2022-05-18T17:38:44+01:00 New Revision: 4739176fd3047dfa13eae307c56c6dba7b605019
URL: https://github.com/llvm/llvm-project/commit/4739176fd3047dfa13eae307c56c6dba7b605019 DIFF: https://github.com/llvm/llvm-project/commit/4739176fd3047dfa13eae307c56c6dba7b605019.diff LOG: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return. Fixes https://github.com/llvm/llvm-project/issues/55557 Reviewed By: LegalizeAdulthood Differential Revision: https://reviews.llvm.org/D125877 Added: Modified: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index 2300b4260c600..1db440120aa3a 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -236,41 +236,6 @@ static std::string replacementExpression(const ASTContext &Context, 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 = @@ -502,8 +467,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> { 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)) { @@ -523,7 +488,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> { ThenReturnBool.Bool != TrailingReturnBool.Bool) { Check->replaceCompoundReturnWithCondition( Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool, - SubIf); + SubIf, ThenReturnBool.Item); } } } @@ -689,11 +654,11 @@ void SimplifyBooleanExprCheck::replaceWithReturnCondition( 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); } diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h index 9a362d7bcc3f8..2724c9a4eddd7 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h @@ -55,7 +55,8 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck { 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, diff --git a/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp index 19339e740bd59..a0a7f52ccf7f1 100644 --- a/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp @@ -2,6 +2,7 @@ #include "ClangTidyTest.h" #include "readability/BracesAroundStatementsCheck.h" #include "readability/NamespaceCommentCheck.h" +#include "readability/SimplifyBooleanExprCheck.h" #include "gtest/gtest.h" namespace clang { @@ -10,6 +11,7 @@ namespace test { using readability::BracesAroundStatementsCheck; using readability::NamespaceCommentCheck; +using readability::SimplifyBooleanExprCheck; using namespace ast_matchers; // Copied from ASTMatchersTests @@ -533,6 +535,16 @@ TEST(BracesAroundStatementsCheckTest, ImplicitCastInReturn) { 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
