llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Mike Crowe (mikecrowe) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/99021.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp (+6-8) - (modified) clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp (-4) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp (+3-3) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp (+24-2) ``````````diff 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); } `````````` </details> https://github.com/llvm/llvm-project/pull/99021 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
