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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:37
+
+  if (PrintfLikeFunctions.empty() && FprintfLikeFunctions.empty()) {
+    PrintfLikeFunctions.push_back("::printf");
----------------
those 2 looks in-depended. Cannot they be put as default value into Option config ?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:70
+void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
----------------
call this function only if PrintfLikeFunctions is not empty.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:78
+
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
----------------
call this function only if FprintfLikeFunctions is not empty


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:79
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
+                          matchers::matchesAnyListedName(FprintfLikeFunctions))
----------------
this could match also methods, maybe something `unless(cxxMethodDecl())` ?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:82
+                          .bind("func_decl")),
+               hasArgument(1, stringLiteral()))
+          .bind("fprintf"),
----------------
consider moving this stringLiteral to be first, consider adding something like `unless(argumentCountIs(0)), unless(argumentCountIs(1))`` at begining or create matcher that verify first that argument count is >= 2, so we could exlude most of callExpr at begining, even before we match functions


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:58
+- ``printf``, ``fprintf``, ``absl::PrintF``, ``absl::FPrintF`` and any
+  functions specified by the ``PrintfLikeFunctions`` option or
+  ``FprintfLikeFunctions`` are replaced with the function specified by the
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:59
+  functions specified by the ``PrintfLikeFunctions`` option or
+  ``FprintfLikeFunctions`` are replaced with the function specified by the
+  ``ReplacementPrintlnFunction`` option if the format string ends with
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:60
+  ``FprintfLikeFunctions`` are replaced with the function specified by the
+  ``ReplacementPrintlnFunction`` option if the format string ends with
+  ``\n`` or ``ReplacementPrintFunction`` otherwise.
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:61
+  ``ReplacementPrintlnFunction`` option if the format string ends with
+  ``\n`` or ``ReplacementPrintFunction`` otherwise.
+- the format string is rewritten to use the ``std::formatter`` language and
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149280