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: [[@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() {
Index: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
+++ clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
@@ -67,6 +67,11 @@
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 @@
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;
Index: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
+++ clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
@@ -340,6 +340,22 @@
}
}
+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 @@
emitFieldWidth(FS, FormatSpec);
emitPrecision(FS, FormatSpec);
+ maybeRotateArguments(FS);
if (!emitType(FS, Arg, FormatSpec))
return false;
@@ -682,5 +699,22 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
