[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