[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:79
+
+.. option:: StrictMode
+
----------------
It turns out that absl::PrintF and absl::FPrintF work like std::format, fmt::printf, etc. and use the signedness of the argument rather than the signedness indicated in the format string to determine how to format the number. In other words:
`unsigned int ui = 0xffffffff; absl::PrintF("%d\n", ui);` yields `4294967295` (see https://godbolt.org/z/dYcbehxP9 ), so the casts that StrictMode adds would change the behaviour of the converted code.

I can think of several ways around this:

1. Update the documentation to explain this, recommending not to use StrictMode when converting Abseil functions.

2. Remove built-in support for Abseil functions from this check. Anyone wishing to convert them can do so via the customisation features, and can choose not to use StrictMode. (This could be documented here.)

3. Teach the code to recognise whether the arguments is being passed as a C-style variable argument list or as fully-typed arguments to a templated function and make StrictMode only add the casts for the former. (I've not investigated how feasible this is.)

4. Treat the known Abseil functions in this check differently by name and disable the casting behaviour. This means that conversion from fmt::printf via the customisation mechanism wouldn't automatically get that behaviour.

As someone who doesn't use Abseil I'm inclined towards option 2, with the possibility of implementing option 3 in a separate commit later. I'm not particularly keen on the other two options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149280/new/

https://reviews.llvm.org/D149280