[clang-tools-extra] 666d224 - [clang-tidy] Fix modernize-use-std-print/format for fmt (#99021)

via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 17 08:45:51 PDT 2024


Author: Mike Crowe
Date: 2024-07-17T17:45:47+02:00
New Revision: 666d224248707f373577b5b049b5b0229100006c

URL: https://github.com/llvm/llvm-project/commit/666d224248707f373577b5b049b5b0229100006c
DIFF: https://github.com/llvm/llvm-project/commit/666d224248707f373577b5b049b5b0229100006c.diff

LOG: [clang-tidy] Fix modernize-use-std-print/format for fmt (#99021)

When fixing #92896 in 0e62d5cf55479981da5e05e406bbca4afb3cdc4f (#94104)
I failed to spot that I'd broken converting from fmt::printf,
fmt::fprintf and fmt::sprintf in these checks since the format parameter
of those functions is not a simple character pointer.

The first part of the previous fix to avoid the assert and instead
produce an error message was sufficient. It was only the second part
that required the format parameter of the called function to be a simple
character pointer that was problematic. Let's remove that second part
and add the now-expected error messages to the lit tests along with
fixing the prototype for the fmt functions to more accurately reflect
the ones used by the fmt library so they are actually useful.

Fixes #92896

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
    clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
    clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
    clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
index d082faa786b37..6cef21f1318a2 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
@@ -47,15 +47,13 @@ void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM,
 }
 
 void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
-  auto CharPointerType =
-      hasType(pointerType(pointee(matchers::isSimpleChar())));
   Finder->addMatcher(
-      callExpr(
-          argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())),
-          callee(functionDecl(
-                     unless(cxxMethodDecl()), hasParameter(0, CharPointerType),
-                     matchers::matchesAnyListedName(StrFormatLikeFunctions))
-                     .bind("func_decl")))
+      callExpr(argumentCountAtLeast(1),
+               hasArgument(0, stringLiteral(isOrdinary())),
+               callee(functionDecl(unless(cxxMethodDecl()),
+                                   matchers::matchesAnyListedName(
+                                       StrFormatLikeFunctions))
+                          .bind("func_decl")))
           .bind("strformat"),
       this);
 }

diff  --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
index 1ea170c3cd310..ff990feadc0c1 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
@@ -95,15 +95,12 @@ unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) {
 }
 
 void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) {
-  auto CharPointerType =
-      hasType(pointerType(pointee(matchers::isSimpleChar())));
   if (!PrintfLikeFunctions.empty())
     Finder->addMatcher(
         unusedReturnValue(
             callExpr(argumentCountAtLeast(1),
                      hasArgument(0, stringLiteral(isOrdinary())),
                      callee(functionDecl(unless(cxxMethodDecl()),
-                                         hasParameter(0, CharPointerType),
                                          matchers::matchesAnyListedName(
                                              PrintfLikeFunctions))
                                 .bind("func_decl")))
@@ -116,7 +113,6 @@ void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) {
             callExpr(argumentCountAtLeast(2),
                      hasArgument(1, stringLiteral(isOrdinary())),
                      callee(functionDecl(unless(cxxMethodDecl()),
-                                         hasParameter(1, CharPointerType),
                                          matchers::matchesAnyListedName(
                                              FprintfLikeFunctions))
                                 .bind("func_decl")))

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
index c025113055cce..7da0bb02ad766 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
@@ -63,4 +63,5 @@ std::string unsupported_format_parameter_type()
   // No fixes here because the format parameter of the function called is not a
   // string.
   return bad_format_type_strprintf("");
+// CHECK-MESSAGES: [[@LINE-1]]:10: warning: unable to use 'fmt::format' instead of 'bad_format_type_strprintf' because first argument is not a narrow string literal [modernize-use-std-format]
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
index 9d136cf309168..1eaf18ac11996 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
@@ -12,9 +12,9 @@
 
 namespace fmt
 {
-// Use const char * for the format since the real type is hard to mock up.
-template <typename... Args>
-std::string sprintf(const char *format, const Args&... args);
+template <typename S, typename... T,
+          typename Char = char>
+std::basic_string<Char> sprintf(const S& fmt, const T&... args);
 } // namespace fmt
 
 std::string fmt_sprintf_simple() {

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp
index 09720001ab837..687b8c0780b01 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp
@@ -1,8 +1,8 @@
 // RUN: %check_clang_tidy -std=c++23 %s modernize-use-std-print %t -- \
 // RUN:   -config="{CheckOptions: \
 // RUN:             { \
-// RUN:               modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2; bad_format_type_printf', \
-// RUN:               modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; bad_format_type_fprintf' \
+// RUN:               modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2; bad_format_type_printf; fmt::printf', \
+// RUN:               modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; bad_format_type_fprintf; fmt::fprintf' \
 // RUN:             } \
 // RUN:            }" \
 // RUN:   -- -isystem %clang_tidy_headers
@@ -106,5 +106,27 @@ void unsupported_format_parameter_type()
   // No fixes here because the format parameter of the function called is not a
   // string.
   bad_format_type_printf("Hello %s", "world");
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'bad_format_type_printf' because first argument is not a narrow string literal [modernize-use-std-print]
+
   bad_format_type_fprintf(stderr, "Hello %s", "world");
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'bad_format_type_fprintf' because first argument is not a narrow string literal [modernize-use-std-print]
+}
+
+namespace fmt {
+  template <typename S, typename... T>
+  inline int printf(const S& fmt, const T&... args);
+
+  template <typename S, typename... T>
+  inline int fprintf(std::FILE* f, const S& fmt, const T&... args);
+}
+
+void fmt_printf()
+{
+  fmt::printf("fmt::printf templated %s argument %d\n", "format", 424);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("fmt::printf templated {} argument {}", "format", 424);
+
+  fmt::fprintf(stderr, "fmt::fprintf templated %s argument %d\n", "format", 425);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println(stderr, "fmt::fprintf templated {} argument {}", "format", 425);
 }


        


More information about the cfe-commits mailing list