llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) <details> <summary>Changes</summary> Fixes: #<!-- -->93157 --- Full diff: https://github.com/llvm/llvm-project/pull/96917.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+35-7) - (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h (+1-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp (+17) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index fd4730d9c8b9c..499c88ef5d4e4 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -9,6 +9,7 @@ #include "SimplifyBooleanExprCheck.h" #include "clang/AST/Expr.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/DiagnosticIDs.h" #include "clang/Lex/Lexer.h" #include "llvm/Support/SaveAndRestore.h" @@ -702,7 +703,8 @@ bool SimplifyBooleanExprCheck::canBeBypassed(const Stmt *S) const { return IgnoreMacros && S->getBeginLoc().isMacroID(); } -void SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context, +/// @brief return true when replacement created. +bool SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context, SourceLocation Loc, StringRef Description, SourceRange ReplacementRange, @@ -712,8 +714,10 @@ void SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context, Context.getSourceManager(), getLangOpts()); DiagnosticBuilder Diag = diag(Loc, Description); - if (!containsDiscardedTokens(Context, CharRange)) + const bool HasReplacement = !containsDiscardedTokens(Context, CharRange); + if (HasReplacement) Diag << FixItHint::CreateReplacement(CharRange, Replacement); + return HasReplacement; } void SimplifyBooleanExprCheck::replaceWithThenStatement( @@ -751,8 +755,18 @@ void SimplifyBooleanExprCheck::replaceWithReturnCondition( replacementExpression(Context, Negated, If->getCond()); std::string Replacement = ("return " + Condition + Terminator).str(); SourceLocation Start = BoolLiteral->getBeginLoc(); - issueDiag(Context, Start, SimplifyConditionalReturnDiagnostic, - If->getSourceRange(), Replacement); + + const bool HasReplacement = + issueDiag(Context, Start, SimplifyConditionalReturnDiagnostic, + If->getSourceRange(), Replacement); + + if (!HasReplacement) { + const SourceRange ConditionRange = If->getCond()->getSourceRange(); + if (ConditionRange.isValid()) + diag(ConditionRange.getBegin(), "conditions that can be simplified", + DiagnosticIDs::Note) + << ConditionRange; + } } void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition( @@ -760,9 +774,23 @@ void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition( const IfStmt *If, const Expr *ThenReturn) { const std::string Replacement = "return " + replacementExpression(Context, Negated, If->getCond()); - issueDiag(Context, ThenReturn->getBeginLoc(), - SimplifyConditionalReturnDiagnostic, - SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement); + + const bool HasReplacement = issueDiag( + Context, ThenReturn->getBeginLoc(), SimplifyConditionalReturnDiagnostic, + SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement); + + if (!HasReplacement) { + const SourceRange ConditionRange = If->getCond()->getSourceRange(); + if (ConditionRange.isValid()) + diag(ConditionRange.getBegin(), "conditions that can be simplified", + DiagnosticIDs::Note) + << ConditionRange; + const SourceRange ReturnRange = Ret->getSourceRange(); + if (ReturnRange.isValid()) + diag(ReturnRange.getBegin(), "return statement that can be simplified", + DiagnosticIDs::Note) + << ReturnRange; + } } void SimplifyBooleanExprCheck::replaceWithAssignment(const ASTContext &Context, diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h index 63c3caa01e01a..2ea6968798408 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h @@ -60,7 +60,7 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck { const BinaryOperator *Inner, bool TryOfferFix, const Stmt *Parent, const ParenExpr *Parens); - void issueDiag(const ASTContext &Context, SourceLocation Loc, + bool issueDiag(const ASTContext &Context, SourceLocation Loc, StringRef Description, SourceRange ReplacementRange, StringRef Replacement); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7fcf5cfbe3783..4df4daff7cf1e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -452,7 +452,8 @@ Changes in existing checks - Improved :doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>` check to avoid to emit - warning for macro when IgnoreMacro option is enabled. + warning for macro when IgnoreMacro option is enabled and improve messages + when auto-fix does not work. - Improved :doc:`readability-static-definition-in-anonymous-namespace <clang-tidy/checks/readability/static-definition-in-anonymous-namespace>` diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp index c14438aa93801..bad1055a01904 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp @@ -353,6 +353,23 @@ bool conditional_return_statements(int i) { // CHECK-FIXES: {{^}} return i == 0;{{$}} // CHECK-FIXES-NEXT: {{^}$}} +bool conditional_return_statements_no_fix_1(int i) { + if (i == 0) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: {{.*}} in conditional return statement + // CHECK-MESSAGES: :[[@LINE-2]]:7: note: conditions that can be simplified + // comment + return false; + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: return statement that can be simplified +} + +bool conditional_return_statements_no_fix_2(int i) { + if (i == 0) return true; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: {{.*}} in conditional return statement + // CHECK-MESSAGES: :[[@LINE-2]]:7: note: conditions that can be simplified + // comment + else return false; +} + bool conditional_return_statements_then_expr(int i, int j) { if (i == j) return (i == 0); else return false; } `````````` </details> https://github.com/llvm/llvm-project/pull/96917 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
