[clang-tools-extra] [clang-tidy] Preserve comments in `readability-use-std-min-max` (PR #169908)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 3 08:37:57 PST 2025
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/169908
>From 3b7d92ba5fb8d5849e68369fc0f8ea56d1cf494d Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Fri, 28 Nov 2025 20:06:44 +0800
Subject: [PATCH 1/5] [clang-tidy] Preserve comments in
`readability-use-std-min-max`
---
.../readability/UseStdMinMaxCheck.cpp | 23 ++++++++++++-------
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++++
.../checkers/readability/use-std-min-max.cpp | 11 +++++++++
3 files changed, 31 insertions(+), 8 deletions(-)
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;
+ }
+}
>From 4674ddcc4d993fb677a1bf3d6ac51a9d738caf52 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 3 Dec 2025 15:19:57 +0800
Subject: [PATCH 2/5] Fix
---
.../readability/UseStdMinMaxCheck.cpp | 50 ++++-
.../checkers/readability/use-std-min-max.cpp | 173 +++++++++++++-----
2 files changed, 170 insertions(+), 53 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index f78e62fbc3c27..a7a439cf3e5bb 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -115,7 +115,8 @@ createReplacement(const Expr *CondLhs, const Expr *CondRhs,
(!GlobalImplicitCastType.isNull()
? "<" + GlobalImplicitCastType.getAsString() + ">("
: "(") +
- CondLhsStr + ", " + CondRhsStr + ");" + Comment)
+ CondLhsStr + ", " + CondRhsStr + ");" + (Comment.empty() ? "" : " ") +
+ Comment)
.str();
}
@@ -171,14 +172,45 @@ 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();
+ llvm::SmallString<64> Comment;
+
+ const auto AppendNormalized = [&](llvm::StringRef Text) {
+ Text = Text.ltrim();
+ if (!Text.empty()) {
+ if (!Comment.empty())
+ Comment += " ";
+ Comment += Text;
+ }
+ };
+
+ AppendNormalized(Lexer::getSourceText(
+ CharSourceRange::getCharRange(
+ Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO),
+ If->getThen()->getBeginLoc()),
+ Source, LO));
+
+ if (const auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
+ const Stmt *Inner = CS->body_front();
+ AppendNormalized(Lexer::getSourceText(
+ CharSourceRange::getCharRange(
+ Lexer::getLocForEndOfToken(CS->getBeginLoc(), 0, Source, LO),
+ Inner->getBeginLoc()),
+ Source, LO));
+
+ llvm::StringRef PostInner = Lexer::getSourceText(
+ CharSourceRange::getCharRange(
+ Lexer::getLocForEndOfToken(Inner->getEndLoc(), 0, Source, LO),
+ CS->getEndLoc()),
+ Source, LO);
+
+ const size_t Semi = PostInner.find(';');
+ if (Semi != llvm::StringRef::npos &&
+ PostInner.take_front(Semi).trim().empty()) {
+ PostInner = PostInner.drop_front(Semi + 1);
+ }
+ AppendNormalized(PostInner);
+ }
+
diag(IfLocation, "use `%0` instead of `%1`")
<< FunctionName << BinaryOp->getOpcodeStr()
<< FixItHint::CreateReplacement(
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 f51bf1ae98ed1..afeffb2604115 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
@@ -12,7 +12,7 @@ constexpr int myConstexprMax(int a, int b) {
#define MY_IF_MACRO(condition, statement) \
if (condition) { \
statement \
- }
+ }
class MyClass {
public:
@@ -28,22 +28,22 @@ void foo(T value7) {
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, value2);
if (value1 < value2)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value1, value2);
if (value1 < value2)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value2, value1);
if (value2 > value1)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value2, value1);
if (value2 > value1)
- value1 = value2;
+ value1 = value2;
// No suggestion needed here
if (value1 == value2)
@@ -53,102 +53,102 @@ void foo(T value7) {
// CHECK-FIXES: value1 = std::max<int>(value1, value4);
short value4;
if(value1<value4)
- value1=value4;
-
+ value1=value4;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value3 = std::min(value1+value2, value3);
if(value1+value2<value3)
- value3 = value1+value2;
-
+ value3 = value1+value2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, myConstexprMin(value2, value3));
if (value1 < myConstexprMin(value2, value3))
- value1 = myConstexprMin(value2, value3);
-
+ value1 = myConstexprMin(value2, value3);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
if (value1 > myConstexprMax(value2, value3))
- value1 = myConstexprMax(value2, value3);
-
+ value1 = myConstexprMax(value2, value3);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value1, value2);
if (value1 <= value2)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, value2);
if (value1 <= value2)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value2, value1);
if (value2 >= value1)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value2, value1);
if (value2 >= value1)
- value2 = value1;
-
+ value2 = value1;
+
// CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
MyClass obj;
if (obj.member1 < obj.member2)
- obj.member1 = obj.member2;
+ obj.member1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
if (obj.member1 < obj.member2)
- obj.member2 = obj.member1;
+ obj.member2 = obj.member1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
if (obj.member2 > obj.member1)
- obj.member2 = obj.member1;
+ obj.member2 = obj.member1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
if (obj.member2 > obj.member1)
- obj.member1 = obj.member2;
-
+ obj.member1 = obj.member2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max<int>(obj.member1, value4);
if (obj.member1 < value4)
- obj.member1 = value4;
-
+ obj.member1 = value4;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
if (obj.member1 + value2 < value3)
- value3 = obj.member1 + value2;
-
+ value3 = obj.member1 + value2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
if (value1 <= obj.member2)
- obj.member2 = value1;
+ obj.member2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, obj.member2);
if (value1 <= obj.member2)
- value1 = obj.member2;
+ value1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(obj.member2, value1);
if (obj.member2 >= value1)
- value1 = obj.member2;
+ value1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
if (obj.member2 >= value1)
- obj.member2 = value1;
-
+ obj.member2 = value1;
+
// No suggestion needed here
if (MY_MACRO_MIN(value1, value2) < value3)
- value3 = MY_MACRO_MIN(value1, value2);
-
+ value3 = MY_MACRO_MIN(value1, value2);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value4 = std::max<int>(value4, value2);
if (value4 < value2){
- value4 = value2;
+ value4 = value2;
}
// No suggestion needed here
@@ -156,18 +156,18 @@ void foo(T value7) {
value2 = value1;
else
value2 = value3;
-
+
// No suggestion needed here
if(value1<value2){
- value2 = value1;
+ value2 = value1;
}
else{
- value2 = value3;
+ value2 = value3;
}
// No suggestion needed here
if(value1<value2){
- value2 = value1;
+ value2 = value1;
int res = value1 + value2;
}
@@ -202,7 +202,7 @@ void foo(T value7) {
value2 = value3;
}
}
-
+
// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value6 = std::min<unsigned int>(value5, value6);
unsigned int value5;
@@ -221,7 +221,7 @@ void foo(T value7) {
const int value8 = 5;
if(value8<value1)
value1 = value8;
-
+
//CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
//CHECK-FIXES: value1 = std::min(value9, value1);
volatile int value9 = 6;
@@ -274,13 +274,98 @@ void useRight() {
} // namespace gh121676
-void testComment() {
+void testComments() {
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
+ // CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
+ if (box_depth > max_depth) // here
{
box_depth = max_depth;
}
+
+ // 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); /* here */
+ if (box_depth > max_depth) /* here */
+ {
+ box_depth = max_depth;
+ }
+
+ // 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); // here
+ if (box_depth > max_depth)
+ // here
+ {
+ box_depth = max_depth;
+ }
+
+ // 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); /* here */
+ if (box_depth > max_depth)
+ /* here */
+ {
+ box_depth = max_depth;
+ }
+
+ // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
+ // CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here
+ // CHECK-FIXES-NEXT: and here
+ // CHECK-FIXES-NEXT: */
+ if (box_depth > max_depth) /* here
+ and here
+ */
+ {
+ box_depth = max_depth;
+ }
+
+ // 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); /* here */
+ if (box_depth > max_depth) { /* here */
+ box_depth = max_depth;
+ }
+
+ // 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); // here
+ if (box_depth > max_depth) { // here
+ box_depth = max_depth;
+ }
+
+ // 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); /* here */
+ if (box_depth > max_depth) {
+ box_depth = max_depth; /* here */
+ }
+
+ // 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); // here
+ if (box_depth > max_depth) {
+ box_depth = max_depth; // here
+ }
+
+ // 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); /* here */
+ if (box_depth > max_depth) {
+ box_depth = max_depth;
+ /* here */
+ }
+
+ // 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); // here
+ if (box_depth > max_depth) {
+ box_depth = max_depth;
+ // here
+ }
+
+ // CHECK-MESSAGES: :[[@LINE+5]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
+ // CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
+ // CHECK-FIXES-NEXT: // and
+ // CHECK-FIXES-NEXT: /* there
+ // CHECK-FIXES-NEXT: again*/
+ if (box_depth > max_depth) {
+ // here
+ box_depth = max_depth; // and
+ /* there
+ again*/
+ }
}
>From ec32830862575de04caa34d1badf1e668e1b6fc0 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 3 Dec 2025 19:01:09 +0800
Subject: [PATCH 3/5] ~
---
.../readability/UseStdMinMaxCheck.cpp | 13 +++
.../checkers/readability/use-std-min-max.cpp | 82 +++++++++----------
2 files changed, 54 insertions(+), 41 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index a7a439cf3e5bb..6cf58811fb255 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -183,6 +183,10 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
}
};
+ // Captures:
+ // if (cond) // Comment A
+ // if (cond) /* Comment A */ { ... }
+ // if (cond) /* Comment A */ x = y;
AppendNormalized(Lexer::getSourceText(
CharSourceRange::getCharRange(
Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO),
@@ -191,18 +195,27 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
const Stmt *Inner = CS->body_front();
+
+ // Captures:
+ // if (cond) { // Comment B ... }
+ // if (cond) { /* Comment B */ x = y; }
AppendNormalized(Lexer::getSourceText(
CharSourceRange::getCharRange(
Lexer::getLocForEndOfToken(CS->getBeginLoc(), 0, Source, LO),
Inner->getBeginLoc()),
Source, LO));
+ // Captures:
+ // if (cond) { x = y; // Comment C }
+ // if (cond) { x = y; /* Comment C */ }
llvm::StringRef PostInner = Lexer::getSourceText(
CharSourceRange::getCharRange(
Lexer::getLocForEndOfToken(Inner->getEndLoc(), 0, Source, LO),
CS->getEndLoc()),
Source, LO);
+ // Strip the trailing semicolon to avoid fixes like:
+ // x = std::min(x, y);; // comment
const size_t Semi = PostInner.find(';');
if (Semi != llvm::StringRef::npos &&
PostInner.take_front(Semi).trim().empty()) {
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 afeffb2604115..35570189e1122 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
@@ -12,7 +12,7 @@ constexpr int myConstexprMax(int a, int b) {
#define MY_IF_MACRO(condition, statement) \
if (condition) { \
statement \
- }
+ }
class MyClass {
public:
@@ -28,22 +28,22 @@ void foo(T value7) {
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, value2);
if (value1 < value2)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value1, value2);
if (value1 < value2)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value2, value1);
if (value2 > value1)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value2, value1);
if (value2 > value1)
- value1 = value2;
+ value1 = value2;
// No suggestion needed here
if (value1 == value2)
@@ -53,102 +53,102 @@ void foo(T value7) {
// CHECK-FIXES: value1 = std::max<int>(value1, value4);
short value4;
if(value1<value4)
- value1=value4;
-
+ value1=value4;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value3 = std::min(value1+value2, value3);
if(value1+value2<value3)
- value3 = value1+value2;
-
+ value3 = value1+value2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, myConstexprMin(value2, value3));
if (value1 < myConstexprMin(value2, value3))
- value1 = myConstexprMin(value2, value3);
-
+ value1 = myConstexprMin(value2, value3);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
if (value1 > myConstexprMax(value2, value3))
- value1 = myConstexprMax(value2, value3);
-
+ value1 = myConstexprMax(value2, value3);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value1, value2);
if (value1 <= value2)
- value2 = value1;
+ value2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, value2);
if (value1 <= value2)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value2, value1);
if (value2 >= value1)
- value1 = value2;
+ value1 = value2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value2 = std::min(value2, value1);
if (value2 >= value1)
- value2 = value1;
-
+ value2 = value1;
+
// CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
MyClass obj;
if (obj.member1 < obj.member2)
- obj.member1 = obj.member2;
+ obj.member1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
if (obj.member1 < obj.member2)
- obj.member2 = obj.member1;
+ obj.member2 = obj.member1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
if (obj.member2 > obj.member1)
- obj.member2 = obj.member1;
+ obj.member2 = obj.member1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
if (obj.member2 > obj.member1)
- obj.member1 = obj.member2;
-
+ obj.member1 = obj.member2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: obj.member1 = std::max<int>(obj.member1, value4);
if (obj.member1 < value4)
- obj.member1 = value4;
-
+ obj.member1 = value4;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
if (obj.member1 + value2 < value3)
- value3 = obj.member1 + value2;
-
+ value3 = obj.member1 + value2;
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
if (value1 <= obj.member2)
- obj.member2 = value1;
+ obj.member2 = value1;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(value1, obj.member2);
if (value1 <= obj.member2)
- value1 = obj.member2;
+ value1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: value1 = std::max(obj.member2, value1);
if (obj.member2 >= value1)
- value1 = obj.member2;
+ value1 = obj.member2;
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
// CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
if (obj.member2 >= value1)
- obj.member2 = value1;
-
+ obj.member2 = value1;
+
// No suggestion needed here
if (MY_MACRO_MIN(value1, value2) < value3)
- value3 = MY_MACRO_MIN(value1, value2);
-
+ value3 = MY_MACRO_MIN(value1, value2);
+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value4 = std::max<int>(value4, value2);
if (value4 < value2){
- value4 = value2;
+ value4 = value2;
}
// No suggestion needed here
@@ -156,18 +156,18 @@ void foo(T value7) {
value2 = value1;
else
value2 = value3;
-
+
// No suggestion needed here
if(value1<value2){
- value2 = value1;
+ value2 = value1;
}
else{
- value2 = value3;
+ value2 = value3;
}
// No suggestion needed here
if(value1<value2){
- value2 = value1;
+ value2 = value1;
int res = value1 + value2;
}
@@ -202,7 +202,7 @@ void foo(T value7) {
value2 = value3;
}
}
-
+
// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
// CHECK-FIXES: value6 = std::min<unsigned int>(value5, value6);
unsigned int value5;
@@ -221,7 +221,7 @@ void foo(T value7) {
const int value8 = 5;
if(value8<value1)
value1 = value8;
-
+
//CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
//CHECK-FIXES: value1 = std::min(value9, value1);
volatile int value9 = 6;
>From ed53afd4e5fa090560b12c1e67acb3127f400f9e Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 3 Dec 2025 22:52:27 +0800
Subject: [PATCH 4/5] ~
---
.../readability/UseStdMinMaxCheck.cpp | 28 +++++++++----------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index 6cf58811fb255..d68686f6fe471 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -183,15 +183,20 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
}
};
+ const auto GetSourceText = [&](SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ return Lexer::getSourceText(
+ CharSourceRange::getCharRange(
+ Lexer::getLocForEndOfToken(StartLoc, 0, Source, LO), EndLoc),
+ Source, LO);
+ };
+
// Captures:
// if (cond) // Comment A
// if (cond) /* Comment A */ { ... }
// if (cond) /* Comment A */ x = y;
- AppendNormalized(Lexer::getSourceText(
- CharSourceRange::getCharRange(
- Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO),
- If->getThen()->getBeginLoc()),
- Source, LO));
+ AppendNormalized(
+ GetSourceText(If->getRParenLoc(), If->getThen()->getBeginLoc()));
if (const auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
const Stmt *Inner = CS->body_front();
@@ -199,20 +204,13 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
// Captures:
// if (cond) { // Comment B ... }
// if (cond) { /* Comment B */ x = y; }
- AppendNormalized(Lexer::getSourceText(
- CharSourceRange::getCharRange(
- Lexer::getLocForEndOfToken(CS->getBeginLoc(), 0, Source, LO),
- Inner->getBeginLoc()),
- Source, LO));
+ AppendNormalized(GetSourceText(CS->getBeginLoc(), Inner->getBeginLoc()));
// Captures:
// if (cond) { x = y; // Comment C }
// if (cond) { x = y; /* Comment C */ }
- llvm::StringRef PostInner = Lexer::getSourceText(
- CharSourceRange::getCharRange(
- Lexer::getLocForEndOfToken(Inner->getEndLoc(), 0, Source, LO),
- CS->getEndLoc()),
- Source, LO);
+ llvm::StringRef PostInner =
+ GetSourceText(Inner->getEndLoc(), CS->getEndLoc());
// Strip the trailing semicolon to avoid fixes like:
// x = std::min(x, y);; // comment
>From def2ccaed9a11e8f68a6642190018ce23e272f56 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Thu, 4 Dec 2025 00:35:13 +0800
Subject: [PATCH 5/5] ~
---
.../clang-tidy/readability/UseStdMinMaxCheck.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index d68686f6fe471..ee0810b45559d 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -202,7 +202,9 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
const Stmt *Inner = CS->body_front();
// Captures:
- // if (cond) { // Comment B ... }
+ // if (cond) { // Comment B
+ // ...
+ // }
// if (cond) { /* Comment B */ x = y; }
AppendNormalized(GetSourceText(CS->getBeginLoc(), Inner->getBeginLoc()));
More information about the cfe-commits
mailing list