llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: mitchell (zeyi2) <details> <summary>Changes</summary> Closes [#<!-- -->121613](https://github.com/llvm/llvm-project/issues/121613) --- Full diff: https://github.com/llvm/llvm-project/pull/169908.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp (+15-8) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp (+11) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp index 5a7add88d6eeb..f78e62fbc3c27 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -96,12 +96,11 @@ static QualType getReplacementCastType(const Expr *CondLhs, const Expr *CondRhs, return GlobalImplicitCastType; } -static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs, - const Expr *AssignLhs, - const SourceManager &Source, - const LangOptions &LO, - StringRef FunctionName, - const BinaryOperator *BO) { +static std::string +createReplacement(const Expr *CondLhs, const Expr *CondRhs, + const Expr *AssignLhs, const SourceManager &Source, + const LangOptions &LO, StringRef FunctionName, + const BinaryOperator *BO, StringRef Comment = "") { const llvm::StringRef CondLhsStr = Lexer::getSourceText( Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO); const llvm::StringRef CondRhsStr = Lexer::getSourceText( @@ -116,7 +115,7 @@ static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs, (!GlobalImplicitCastType.isNull() ? "<" + GlobalImplicitCastType.getAsString() + ">(" : "(") + - CondLhsStr + ", " + CondRhsStr + ");") + CondLhsStr + ", " + CondRhsStr + ");" + Comment) .str(); } @@ -172,13 +171,21 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { auto ReplaceAndDiagnose = [&](const llvm::StringRef FunctionName) { const SourceManager &Source = *Result.SourceManager; + const std::string Comment = + Lexer::getSourceText( + CharSourceRange::getCharRange( + Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO), + If->getThen()->getBeginLoc()), + Source, LO) + .rtrim() + .str(); diag(IfLocation, "use `%0` instead of `%1`") << FunctionName << BinaryOp->getOpcodeStr() << FixItHint::CreateReplacement( SourceRange(IfLocation, Lexer::getLocForEndOfToken( ThenLocation, 0, Source, LO)), createReplacement(CondLhs, CondRhs, AssignLhs, Source, LO, - FunctionName, BinaryOp)) + FunctionName, BinaryOp, Comment)) << IncludeInserter.createIncludeInsertion( Source.getFileID(If->getBeginLoc()), AlgorithmHeader); }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a6f80e3721db1..d2da19b61132c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -555,6 +555,11 @@ Changes in existing checks <clang-tidy/checks/readability/use-concise-preprocessor-directives>` check to generate correct fix-its for forms without a space after the directive. +- Improved :doc:`readability-use-std-min-max + <clang-tidy/checks/readability/use-std-min-max>` check by ensuring that + comments between the ``if`` condition and the ``then`` block are preserved + when applying the fix. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp index 35ade8a7c6d37..f51bf1ae98ed1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -273,3 +273,14 @@ void useRight() { } } // namespace gh121676 + +void testComment() { + int box_depth = 10; + int max_depth = 5; + // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max] + // CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // limit the depth to max + if (box_depth > max_depth) // limit the depth to max + { + box_depth = max_depth; + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/169908 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
