[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
This commit is contained in:
Nathan James 2022-05-18 17:38:42 +01:00
parent 46eef76876
commit 4739176fd3
No known key found for this signature in database
GPG Key ID: CC007AFCDA90AA5F
3 changed files with 20 additions and 42 deletions

View File

@ -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 @@ public:
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 @@ public:
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);
}

View File

@ -55,7 +55,8 @@ private:
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,

View File

@ -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