[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check
Nathan James via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 26 23:27:12 PDT 2023
njames93 added a comment.
It may be a good idea to create unittests for the formatconverter in `clang-tools-extra/unittests/clang-tidy/`
Can this check be renamed to just `modernize-use-std-print`. This sticks with the convention of other modernize names and potentially in the future allows us to convert ostream operations to use `std::print`
================
Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp:322
+ if (!isRealCharType(Pointee))
+ ArgFixes.emplace_back(Arg, "reinterpret_cast<const char *>(");
+ }
----------------
By default this check should not do this conversion, we shouldn't be creating UB in fixes emitted by clang-tidy unless the user explicitly opts in.
Maybe a good framework moving forward is to still a warning about converting up std::print, but emit a note here saying why the automatic conversation isn't possible.
================
Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp:412
+ else
+ ArgFixes.emplace_back(Arg, "reinterpret_cast<const void *>(");
+ break;
----------------
`reinterpret_cast` is not needed here, a static cast to`const void*` will work.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/printf-to-std-print.rst:61
+- any arguments where the format string and the parameter differ in
+ signedness will be wrapped in an approprate ``static_cast``. For example,
+ given ``int i = 42``, then ``printf("%u\n", i)`` will become
----------------
Perhaps these conversions should only be done in StrictMode
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/printf-to-std-print.rst:92
+
+It is helpful to use the `readability-redundant-string-cstr
+<../readability/redundant-string-cstr.html>` check after this check to
----------------
It may be wise in a future version to just do that conversion anyway. 2 stage fixes are annoying for users to have to use
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149280/new/
https://reviews.llvm.org/D149280
More information about the cfe-commits
mailing list