Skip to content

Commit 4674ddc

Browse files
committed
Fix
1 parent 3b7d92b commit 4674ddc

File tree

2 files changed

+170
-53
lines changed

2 files changed

+170
-53
lines changed

clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ createReplacement(const Expr *CondLhs, const Expr *CondRhs,
115115
(!GlobalImplicitCastType.isNull()
116116
? "<" + GlobalImplicitCastType.getAsString() + ">("
117117
: "(") +
118-
CondLhsStr + ", " + CondRhsStr + ");" + Comment)
118+
CondLhsStr + ", " + CondRhsStr + ");" + (Comment.empty() ? "" : " ") +
119+
Comment)
119120
.str();
120121
}
121122

@@ -171,14 +172,45 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
171172

172173
auto ReplaceAndDiagnose = [&](const llvm::StringRef FunctionName) {
173174
const SourceManager &Source = *Result.SourceManager;
174-
const std::string Comment =
175-
Lexer::getSourceText(
176-
CharSourceRange::getCharRange(
177-
Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO),
178-
If->getThen()->getBeginLoc()),
179-
Source, LO)
180-
.rtrim()
181-
.str();
175+
llvm::SmallString<64> Comment;
176+
177+
const auto AppendNormalized = [&](llvm::StringRef Text) {
178+
Text = Text.ltrim();
179+
if (!Text.empty()) {
180+
if (!Comment.empty())
181+
Comment += " ";
182+
Comment += Text;
183+
}
184+
};
185+
186+
AppendNormalized(Lexer::getSourceText(
187+
CharSourceRange::getCharRange(
188+
Lexer::getLocForEndOfToken(If->getRParenLoc(), 0, Source, LO),
189+
If->getThen()->getBeginLoc()),
190+
Source, LO));
191+
192+
if (const auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
193+
const Stmt *Inner = CS->body_front();
194+
AppendNormalized(Lexer::getSourceText(
195+
CharSourceRange::getCharRange(
196+
Lexer::getLocForEndOfToken(CS->getBeginLoc(), 0, Source, LO),
197+
Inner->getBeginLoc()),
198+
Source, LO));
199+
200+
llvm::StringRef PostInner = Lexer::getSourceText(
201+
CharSourceRange::getCharRange(
202+
Lexer::getLocForEndOfToken(Inner->getEndLoc(), 0, Source, LO),
203+
CS->getEndLoc()),
204+
Source, LO);
205+
206+
const size_t Semi = PostInner.find(';');
207+
if (Semi != llvm::StringRef::npos &&
208+
PostInner.take_front(Semi).trim().empty()) {
209+
PostInner = PostInner.drop_front(Semi + 1);
210+
}
211+
AppendNormalized(PostInner);
212+
}
213+
182214
diag(IfLocation, "use `%0` instead of `%1`")
183215
<< FunctionName << BinaryOp->getOpcodeStr()
184216
<< FixItHint::CreateReplacement(

clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp

Lines changed: 129 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ constexpr int myConstexprMax(int a, int b) {
1212
#define MY_IF_MACRO(condition, statement) \
1313
if (condition) { \
1414
statement \
15-
}
15+
}
1616

1717
class MyClass {
1818
public:
@@ -28,22 +28,22 @@ void foo(T value7) {
2828
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
2929
// CHECK-FIXES: value1 = std::max(value1, value2);
3030
if (value1 < value2)
31-
value1 = value2;
31+
value1 = value2;
3232

3333
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
3434
// CHECK-FIXES: value2 = std::min(value1, value2);
3535
if (value1 < value2)
36-
value2 = value1;
36+
value2 = value1;
3737

3838
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
3939
// CHECK-FIXES: value2 = std::min(value2, value1);
4040
if (value2 > value1)
41-
value2 = value1;
41+
value2 = value1;
4242

4343
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
4444
// CHECK-FIXES: value1 = std::max(value2, value1);
4545
if (value2 > value1)
46-
value1 = value2;
46+
value1 = value2;
4747

4848
// No suggestion needed here
4949
if (value1 == value2)
@@ -53,121 +53,121 @@ void foo(T value7) {
5353
// CHECK-FIXES: value1 = std::max<int>(value1, value4);
5454
short value4;
5555
if(value1<value4)
56-
value1=value4;
57-
56+
value1=value4;
57+
5858
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
5959
// CHECK-FIXES: value3 = std::min(value1+value2, value3);
6060
if(value1+value2<value3)
61-
value3 = value1+value2;
62-
61+
value3 = value1+value2;
62+
6363
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
6464
// CHECK-FIXES: value1 = std::max(value1, myConstexprMin(value2, value3));
6565
if (value1 < myConstexprMin(value2, value3))
66-
value1 = myConstexprMin(value2, value3);
67-
66+
value1 = myConstexprMin(value2, value3);
67+
6868
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
6969
// CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
7070
if (value1 > myConstexprMax(value2, value3))
71-
value1 = myConstexprMax(value2, value3);
72-
71+
value1 = myConstexprMax(value2, value3);
72+
7373
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
7474
// CHECK-FIXES: value2 = std::min(value1, value2);
7575
if (value1 <= value2)
76-
value2 = value1;
76+
value2 = value1;
7777

7878
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
7979
// CHECK-FIXES: value1 = std::max(value1, value2);
8080
if (value1 <= value2)
81-
value1 = value2;
81+
value1 = value2;
8282

8383
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
8484
// CHECK-FIXES: value1 = std::max(value2, value1);
8585
if (value2 >= value1)
86-
value1 = value2;
86+
value1 = value2;
8787

8888
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
8989
// CHECK-FIXES: value2 = std::min(value2, value1);
9090
if (value2 >= value1)
91-
value2 = value1;
92-
91+
value2 = value1;
92+
9393
// CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
9494
// CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
9595
MyClass obj;
9696
if (obj.member1 < obj.member2)
97-
obj.member1 = obj.member2;
97+
obj.member1 = obj.member2;
9898

9999
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
100100
// CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
101101
if (obj.member1 < obj.member2)
102-
obj.member2 = obj.member1;
102+
obj.member2 = obj.member1;
103103

104104
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
105105
// CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
106106
if (obj.member2 > obj.member1)
107-
obj.member2 = obj.member1;
107+
obj.member2 = obj.member1;
108108

109109
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
110110
// CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
111111
if (obj.member2 > obj.member1)
112-
obj.member1 = obj.member2;
113-
112+
obj.member1 = obj.member2;
113+
114114
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
115115
// CHECK-FIXES: obj.member1 = std::max<int>(obj.member1, value4);
116116
if (obj.member1 < value4)
117-
obj.member1 = value4;
118-
117+
obj.member1 = value4;
118+
119119
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
120120
// CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
121121
if (obj.member1 + value2 < value3)
122-
value3 = obj.member1 + value2;
123-
122+
value3 = obj.member1 + value2;
123+
124124
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max]
125125
// CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
126126
if (value1 <= obj.member2)
127-
obj.member2 = value1;
127+
obj.member2 = value1;
128128

129129
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max]
130130
// CHECK-FIXES: value1 = std::max(value1, obj.member2);
131131
if (value1 <= obj.member2)
132-
value1 = obj.member2;
132+
value1 = obj.member2;
133133

134134
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max]
135135
// CHECK-FIXES: value1 = std::max(obj.member2, value1);
136136
if (obj.member2 >= value1)
137-
value1 = obj.member2;
137+
value1 = obj.member2;
138138

139139
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max]
140140
// CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
141141
if (obj.member2 >= value1)
142-
obj.member2 = value1;
143-
142+
obj.member2 = value1;
143+
144144
// No suggestion needed here
145145
if (MY_MACRO_MIN(value1, value2) < value3)
146-
value3 = MY_MACRO_MIN(value1, value2);
147-
146+
value3 = MY_MACRO_MIN(value1, value2);
147+
148148
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
149149
// CHECK-FIXES: value4 = std::max<int>(value4, value2);
150150
if (value4 < value2){
151-
value4 = value2;
151+
value4 = value2;
152152
}
153153

154154
// No suggestion needed here
155155
if(value1 < value2)
156156
value2 = value1;
157157
else
158158
value2 = value3;
159-
159+
160160
// No suggestion needed here
161161
if(value1<value2){
162-
value2 = value1;
162+
value2 = value1;
163163
}
164164
else{
165-
value2 = value3;
165+
value2 = value3;
166166
}
167167

168168
// No suggestion needed here
169169
if(value1<value2){
170-
value2 = value1;
170+
value2 = value1;
171171
int res = value1 + value2;
172172
}
173173

@@ -202,7 +202,7 @@ void foo(T value7) {
202202
value2 = value3;
203203
}
204204
}
205-
205+
206206
// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
207207
// CHECK-FIXES: value6 = std::min<unsigned int>(value5, value6);
208208
unsigned int value5;
@@ -221,7 +221,7 @@ void foo(T value7) {
221221
const int value8 = 5;
222222
if(value8<value1)
223223
value1 = value8;
224-
224+
225225
//CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
226226
//CHECK-FIXES: value1 = std::min(value9, value1);
227227
volatile int value9 = 6;
@@ -274,13 +274,98 @@ void useRight() {
274274

275275
} // namespace gh121676
276276

277-
void testComment() {
277+
void testComments() {
278278
int box_depth = 10;
279279
int max_depth = 5;
280+
280281
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
281-
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // limit the depth to max
282-
if (box_depth > max_depth) // limit the depth to max
282+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
283+
if (box_depth > max_depth) // here
283284
{
284285
box_depth = max_depth;
285286
}
287+
288+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
289+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here */
290+
if (box_depth > max_depth) /* here */
291+
{
292+
box_depth = max_depth;
293+
}
294+
295+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
296+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
297+
if (box_depth > max_depth)
298+
// here
299+
{
300+
box_depth = max_depth;
301+
}
302+
303+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
304+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here */
305+
if (box_depth > max_depth)
306+
/* here */
307+
{
308+
box_depth = max_depth;
309+
}
310+
311+
// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
312+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here
313+
// CHECK-FIXES-NEXT: and here
314+
// CHECK-FIXES-NEXT: */
315+
if (box_depth > max_depth) /* here
316+
and here
317+
*/
318+
{
319+
box_depth = max_depth;
320+
}
321+
322+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
323+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here */
324+
if (box_depth > max_depth) { /* here */
325+
box_depth = max_depth;
326+
}
327+
328+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
329+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
330+
if (box_depth > max_depth) { // here
331+
box_depth = max_depth;
332+
}
333+
334+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
335+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here */
336+
if (box_depth > max_depth) {
337+
box_depth = max_depth; /* here */
338+
}
339+
340+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
341+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
342+
if (box_depth > max_depth) {
343+
box_depth = max_depth; // here
344+
}
345+
346+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
347+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); /* here */
348+
if (box_depth > max_depth) {
349+
box_depth = max_depth;
350+
/* here */
351+
}
352+
353+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
354+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
355+
if (box_depth > max_depth) {
356+
box_depth = max_depth;
357+
// here
358+
}
359+
360+
// CHECK-MESSAGES: :[[@LINE+5]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max]
361+
// CHECK-FIXES: box_depth = std::min(box_depth, max_depth); // here
362+
// CHECK-FIXES-NEXT: // and
363+
// CHECK-FIXES-NEXT: /* there
364+
// CHECK-FIXES-NEXT: again*/
365+
if (box_depth > max_depth) {
366+
// here
367+
box_depth = max_depth; // and
368+
/* there
369+
again*/
370+
}
286371
}

0 commit comments

Comments
 (0)