This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix width/precision argument order in modernize-use-std-print

Authored by mikecrowe on Jul 1 2023, 9:42 AM.



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.


Diff Detail

Event Timeline

mikecrowe created this revision.Jul 1 2023, 9:42 AM
Herald added a project: Restricted Project. · View Herald Transcript
mikecrowe requested review of this revision.Jul 1 2023, 9:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2023, 9:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
PiotrZSL accepted this revision.Jul 1 2023, 10:17 AM
This revision is now accepted and ready to land.Jul 1 2023, 10:17 AM

Just few nits, from functionally point of view looks fine.

NOTE: You can use std::pair here.

Thanks for the review.


Good point.


True, but in my mind std::pair only exists because std::tuple couldn't be implemented in the C++98.

I was going to change it to use std::pair anyway, but I notice that I already use std::tuple for ArgFixes just above. Should I change both of them?

mikecrowe updated this revision to Diff 536595.Jul 2 2023, 7:57 AM

Use emplace_back rather than push_back(make_tuple())

mikecrowe marked an inline comment as done.Jul 2 2023, 7:57 AM
PiotrZSL added inline comments.Jul 2 2023, 10:11 AM

It will be compiled to same thing anyway. So it's up to you. Keep it consistent.
I personally use std::pair for 2 arguments, and std::tuple for more than 2.
But to be honest probably better would be to introduce some structure so that those two unsigned could be named. And probably same for ArgFixes, but there we got different types, so it's not a big problem.

mikecrowe added inline comments.Jul 2 2023, 10:39 AM

I think that you're probably right, but the names won't be used from the call site since I use structured binding there. Do you mind if I do both together in a separate commit?

PiotrZSL added inline comments.Jul 3 2023, 9:37 AM

Not a problem, lets leave it for a next change.