Skip to content

Conversation

@zeyi2
Copy link
Member

@zeyi2 zeyi2 commented Nov 28, 2025

Closes #121613

@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: mitchell (zeyi2)

Changes

Closes #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 --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;
+  }
+}

@zeyi2 zeyi2 requested a review from vbvictor December 3, 2025 07:33
@zeyi2 zeyi2 requested review from HerrCai0907 and vbvictor December 3, 2025 16:37
Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zeyi2 zeyi2 merged commit 9342d03 into llvm:main Dec 5, 2025
12 checks passed
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang-tidy] readability-use-std-min-max drops comments

4 participants