[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2806cf4b5430: [clang-tidy] Fix width/precision argument
order in modernize-use-std-print (authored by mikecrowe, committed by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154283/new/
https://reviews.llvm.org/D154283
Files:
clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -1024,7 +1024,7 @@
printf("Right-justified integer with field width argument %*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 424242, 5);
printf("Right-justified integer with field width argument %2$*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1061,7 +1061,7 @@
printf("Left-justified integer with field width argument %-*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 424242, 5);
printf("Left-justified integer with field width argument %2$-*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1087,15 +1087,15 @@
printf("Hello %.*f after\n", 10, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:.{}f} after", 10, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:.{}f} after", 3.14159265358979323846, 10);
printf("Hello %10.*f after\n", 3, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3.14159265358979323846, 3);
printf("Hello %*.*f after\n", 10, 4, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 10, 4, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 3.14159265358979323846, 10, 4);
printf("Hello %1$.*2$f after\n", 3.14159265358979323846, 4);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1112,9 +1112,33 @@
}
void printf_field_width_and_precision() {
- printf("Hello %1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ printf("width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+ // CHECK-FIXES: std::println("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
+
+ printf("width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+
+ const int width = 10, precision = 3;
+ printf("width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println("width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
+}
+
+void fprintf_field_width_and_precision() {
+ fprintf(stderr, "width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
+ // CHECK-MESSAGES:
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
PiotrZSL added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector> ArgRotates; + mikecrowe wrote: > PiotrZSL wrote: > > mikecrowe wrote: > > > PiotrZSL wrote: > > > > NOTE: You can use std::pair here. > > > True, but in my mind `std::pair` only exists because `std::tuple` > > > couldn't be implemented in the C++98. > > > > > > I was going to change it to use `std::pair` anyway, but I notice that I > > > already use `std::tuple` for `ArgFixes` just above. Should I change both > > > of them? > > It will be compiled to same thing anyway. So it's up to you. Keep it > > consistent. > > I personally use std::pair for 2 arguments, and std::tuple for more than 2. > > But to be honest probably better would be to introduce some structure so > > that those two `unsigned` could be named. And probably same for ArgFixes, > > but there we got different types, so it's not a big problem. > I think that you're probably right, but the names won't be used from the call > site since I use structured binding there. Do you mind if I do both together > in a separate commit? Not a problem, lets leave it for a next change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
mikecrowe added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector> ArgRotates; + PiotrZSL wrote: > mikecrowe wrote: > > PiotrZSL wrote: > > > NOTE: You can use std::pair here. > > True, but in my mind `std::pair` only exists because `std::tuple` couldn't > > be implemented in the C++98. > > > > I was going to change it to use `std::pair` anyway, but I notice that I > > already use `std::tuple` for `ArgFixes` just above. Should I change both of > > them? > It will be compiled to same thing anyway. So it's up to you. Keep it > consistent. > I personally use std::pair for 2 arguments, and std::tuple for more than 2. > But to be honest probably better would be to introduce some structure so that > those two `unsigned` could be named. And probably same for ArgFixes, but > there we got different types, so it's not a big problem. I think that you're probably right, but the names won't be used from the call site since I use structured binding there. Do you mind if I do both together in a separate commit? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
PiotrZSL added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector> ArgRotates; + mikecrowe wrote: > PiotrZSL wrote: > > NOTE: You can use std::pair here. > True, but in my mind `std::pair` only exists because `std::tuple` couldn't be > implemented in the C++98. > > I was going to change it to use `std::pair` anyway, but I notice that I > already use `std::tuple` for `ArgFixes` just above. Should I change both of > them? It will be compiled to same thing anyway. So it's up to you. Keep it consistent. I personally use std::pair for 2 arguments, and std::tuple for more than 2. But to be honest probably better would be to introduce some structure so that those two `unsigned` could be named. And probably same for ArgFixes, but there we got different types, so it's not a big problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
mikecrowe updated this revision to Diff 536595.
mikecrowe added a comment.
Use emplace_back rather than push_back(make_tuple())
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154283/new/
https://reviews.llvm.org/D154283
Files:
clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -1024,7 +1024,7 @@
printf("Right-justified integer with field width argument %*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 424242, 5);
printf("Right-justified integer with field width argument %2$*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1061,7 +1061,7 @@
printf("Left-justified integer with field width argument %-*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 424242, 5);
printf("Left-justified integer with field width argument %2$-*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1087,15 +1087,15 @@
printf("Hello %.*f after\n", 10, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:.{}f} after", 10, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:.{}f} after", 3.14159265358979323846, 10);
printf("Hello %10.*f after\n", 3, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3.14159265358979323846, 3);
printf("Hello %*.*f after\n", 10, 4, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 10, 4, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 3.14159265358979323846, 10, 4);
printf("Hello %1$.*2$f after\n", 3.14159265358979323846, 4);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1112,9 +1112,33 @@
}
void printf_field_width_and_precision() {
- printf("Hello %1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ printf("width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+ // CHECK-FIXES: std::println("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
+
+ printf("width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+
+ const int width = 10, precision = 3;
+ printf("width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println("width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
+}
+
+void fprintf_field_width_and_precision() {
+ fprintf(stderr, "width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+ // CHECK-FIXES:
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
mikecrowe added a comment. Thanks for the review. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp:356-357 + if (ArgCount) +ArgRotates.push_back( +std::make_tuple(FS.getArgIndex() + ArgsOffset, ArgCount)); +} PiotrZSL wrote: > Good point. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector> ArgRotates; + PiotrZSL wrote: > NOTE: You can use std::pair here. True, but in my mind `std::pair` only exists because `std::tuple` couldn't be implemented in the C++98. I was going to change it to use `std::pair` anyway, but I notice that I already use `std::tuple` for `ArgFixes` just above. Should I change both of them? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
PiotrZSL added a comment. Just few nits, from functionally point of view looks fine. Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp:356-357 + if (ArgCount) +ArgRotates.push_back( +std::make_tuple(FS.getArgIndex() + ArgsOffset, ArgCount)); +} Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector> ArgRotates; + NOTE: You can use std::pair here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154283: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
mikecrowe created this revision.
mikecrowe added a reviewer: PiotrZSL.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
mikecrowe requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Victor Zverovich pointed out[1] that printf takes the field width and
precision arguments before the value to be printed whereas std::print
takes the value first (unless positional arguments are used.) Many of
the test cases in use-std-print.cpp were incorrect.
Teach the check to rotate the arguments when required to correct
this. Correct the test cases and add more.
[1] https://github.com/fmtlib/fmt/pull/3515#issuecomment-1615259893
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154283
Files:
clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -1024,7 +1024,7 @@
printf("Right-justified integer with field width argument %*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Right-justified integer with field width argument {:{}} after", 424242, 5);
printf("Right-justified integer with field width argument %2$*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1061,7 +1061,7 @@
printf("Left-justified integer with field width argument %-*d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 5, 424242);
+ // CHECK-FIXES: std::println("Left-justified integer with field width argument {:<{}} after", 424242, 5);
printf("Left-justified integer with field width argument %2$-*1$d after\n", 5, 424242);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1087,15 +1087,15 @@
printf("Hello %.*f after\n", 10, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:.{}f} after", 10, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:.{}f} after", 3.14159265358979323846, 10);
printf("Hello %10.*f after\n", 3, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:10.{}f} after", 3.14159265358979323846, 3);
printf("Hello %*.*f after\n", 10, 4, 3.14159265358979323846);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 10, 4, 3.14159265358979323846);
+ // CHECK-FIXES: std::println("Hello {:{}.{}f} after", 3.14159265358979323846, 10, 4);
printf("Hello %1$.*2$f after\n", 3.14159265358979323846, 4);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
@@ -1112,9 +1112,33 @@
}
void printf_field_width_and_precision() {
- printf("Hello %1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ printf("width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
- // CHECK-FIXES: std::println("Hello {0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+ // CHECK-FIXES: std::println("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
+
+ printf("width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+
+ const int width = 10, precision = 3;
+ printf("width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
+ //
