[clang-tools-extra] 2806cf4 - [clang-tidy] Fix width/precision argument order in modernize-use-std-print
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 3 09:39:17 PDT 2023
Author: Mike Crowe
Date: 2023-07-03T16:39:04Z
New Revision: 2806cf4b5430ad6d4d5aa2d501aea1de67272876
URL: https://github.com/llvm/llvm-project/commit/2806cf4b5430ad6d4d5aa2d501aea1de67272876
DIFF: https://github.com/llvm/llvm-project/commit/2806cf4b5430ad6d4d5aa2d501aea1de67272876.diff
LOG: [clang-tidy] Fix width/precision argument order in modernize-use-std-print
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
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D154283
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
index f12fa84ec6b141..3e6442c5fd63c1 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
@@ -340,6 +340,22 @@ void FormatStringConverter::emitPrecision(const PrintfSpecifier &FS,
}
}
+void FormatStringConverter::maybeRotateArguments(const PrintfSpecifier &FS) {
+ unsigned ArgCount = 0;
+ const OptionalAmount FieldWidth = FS.getFieldWidth();
+ const OptionalAmount FieldPrecision = FS.getPrecision();
+
+ if (FieldWidth.getHowSpecified() == OptionalAmount::Arg &&
+ !FieldWidth.usesPositionalArg())
+ ++ArgCount;
+ if (FieldPrecision.getHowSpecified() == OptionalAmount::Arg &&
+ !FieldPrecision.usesPositionalArg())
+ ++ArgCount;
+
+ if (ArgCount)
+ ArgRotates.emplace_back(FS.getArgIndex() + ArgsOffset, ArgCount);
+}
+
void FormatStringConverter::emitStringArgument(const Expr *Arg) {
// If the argument is the result of a call to std::string::c_str() or
// data() with a return type of char then we can remove that call and
@@ -531,6 +547,7 @@ bool FormatStringConverter::convertArgument(const PrintfSpecifier &FS,
emitFieldWidth(FS, FormatSpec);
emitPrecision(FS, FormatSpec);
+ maybeRotateArguments(FS);
if (!emitType(FS, Arg, FormatSpec))
return false;
@@ -682,5 +699,22 @@ void FormatStringConverter::applyFixes(DiagnosticBuilder &Diag,
if (!ArgText.empty())
Diag << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
}
+
+ // ArgCount is one less than the number of arguments to be rotated.
+ for (auto [ValueArgIndex, ArgCount] : ArgRotates) {
+ assert(ValueArgIndex < NumArgs);
+ assert(ValueArgIndex > ArgCount);
+
+ // First move the value argument to the right place.
+ Diag << tooling::fixit::createReplacement(*Args[ValueArgIndex - ArgCount],
+ *Args[ValueArgIndex], *Context);
+
+ // Now shift down the field width and precision (if either are present) to
+ // accommodate it.
+ for (size_t Offset = 0; Offset < ArgCount; ++Offset)
+ Diag << tooling::fixit::createReplacement(
+ *Args[ValueArgIndex - Offset], *Args[ValueArgIndex - Offset - 1],
+ *Context);
+ }
}
} // namespace clang::tidy::utils
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
index b246013c24c457..200e530b40810b 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
@@ -67,6 +67,11 @@ class FormatStringConverter
std::vector<std::tuple<const Expr *, std::string>> ArgFixes;
std::vector<clang::ast_matchers::BoundNodes> ArgCStrRemovals;
+ // Argument rotations to cope with the fact that std::print puts the value to
+ // be formatted first and the width and precision afterwards whereas printf
+ // puts the width and preicision first.
+ std::vector<std::tuple<unsigned, unsigned>> ArgRotates;
+
void emitAlignment(const PrintfSpecifier &FS, std::string &FormatSpec);
void emitSign(const PrintfSpecifier &FS, std::string &FormatSpec);
void emitAlternativeForm(const PrintfSpecifier &FS, std::string &FormatSpec);
@@ -81,6 +86,8 @@ class FormatStringConverter
bool convertArgument(const PrintfSpecifier &FS, const Expr *Arg,
std::string &StandardFormatString);
+ void maybeRotateArguments(const PrintfSpecifier &FS);
+
bool HandlePrintfSpecifier(const PrintfSpecifier &FS,
const char *StartSpecifier, unsigned SpecifierLen,
const TargetInfo &Target) override;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
index 437eb83055f43d..4d6bcef3161ac4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -1024,7 +1024,7 @@ void printf_right_justified() {
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 @@ void printf_left_justified() {
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 @@ void printf_precision() {
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_precision() {
}
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: std::println(stderr, "width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
+
+ fprintf(stderr, "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 'fprintf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println(stderr, "width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
+
+ const int width = 10, precision = 3;
+ fprintf(stderr, "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 'fprintf' [modernize-use-std-print]
+ // CHECK-FIXES: std::println(stderr, "width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
}
void printf_alternative_form() {
More information about the cfe-commits
mailing list