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

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat May 6 09:29:38 PDT 2023


njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h:20
+    if (ReplacementPrintFunction == "std::print")
+      return LangOpts.CPlusPlus2b;
+    return LangOpts.CPlusPlus;
----------------
This will need rebasing and changing to `LangOpts.CPlusPlus23`


================
Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp:408-413
+        if (Arg->getType()->isNullPtrType())
+          ; // std::format knows how to format nullptr
+        else if (Arg->getType()->isVoidPointerType())
+          ; // std::format knows how to format void pointers
+        else
+          ArgFixes.emplace_back(Arg, "static_cast<const void *>(");
----------------
Is it not nicer to just search for the compliment


================
Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:57
+
+  virtual bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
+                                     const char *startSpecifier,
----------------
Don't need to specify virtual here when the function is marked override


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:44-45
+
+- It is assumed that the ``<print>`` (or other appropriate) header has
+  already been included. No attempt is made to include it.
+
----------------
This should be relatively easy to address, just look at how other checks use the `IncludeInserter` to add includes.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp:124-126
+  printf("Integer %d from bool\n", b);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("Integer {:d} from bool\n", b);
----------------
It would be nice if the there was support for changing this to `std::println("Integer {:d} from bool", b);`


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