llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Mike Crowe (mikecrowe) <details> <summary>Changes</summary> These checks previously failed with absl::StrFormat and absl::PrintF etc. with: Unable to use 'std::format' instead of 'StrFormat' because first argument is not a narrow string literal [modernize-use-std-format] because FormatStringConverter was rejecting the format string if it had already converted into a different type. Fix the tests so that they check this case properly by accepting string_view rather than const char * and fix the check so that these tests pass. Update the existing tests that checked for the error message that can no longer happen. Fixes: https://github.com/llvm/llvm-project/issues/129484 --- Full diff: https://github.com/llvm/llvm-project/pull/142312.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp (+2-6) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+10) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp (+11-6) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp (+2-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp (+3-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp (+16-9) ``````````diff diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index 7f4ccca84faa5..e1c1bee97f6d4 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -207,13 +207,9 @@ FormatStringConverter::FormatStringConverter( ArgsOffset(FormatArgOffset + 1), LangOpts(LO) { assert(ArgsOffset <= NumArgs); FormatExpr = llvm::dyn_cast<StringLiteral>( - Args[FormatArgOffset]->IgnoreImplicitAsWritten()); + Args[FormatArgOffset]->IgnoreUnlessSpelledInSource()); - if (!FormatExpr || !FormatExpr->isOrdinary()) { - // Function must have a narrow string literal as its first argument. - conversionNotPossible("first argument is not a narrow string literal"); - return; - } + assert(FormatExpr && FormatExpr->isOrdinary()); if (const std::optional<StringRef> MaybeMacroName = formatStringContainsUnreplaceableMacro(Call, FormatExpr, SM, PP); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e0f81a032c38d..3bd601ebdb580 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -232,10 +232,20 @@ Changes in existing checks matched scenarios of ``find`` and ``rfind`` methods and fixing false positives when those methods were called with 3 arguments. +- Improved :doc:`modernize-use-std-print + <clang-tidy/checks/modernize/use-std-print>` check to correctly match + when the format string is converted to a different type by an implicit + constructor call. + - Improved :doc:`modernize-use-std-numbers <clang-tidy/checks/modernize/use-std-numbers>` check to support math functions of different precisions. +- Improved :doc:`modernize-use-std-format + <clang-tidy/checks/modernize/use-std-format>` check to correctly match + when the format string is converted to a different type by an implicit + constructor call. + - Improved :doc:`performance-move-const-arg <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives on ternary operators calling ``std::move``. 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 7da0bb02ad766..0f3458e61856a 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 @@ -2,7 +2,7 @@ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: { \ // RUN: modernize-use-std-format.StrictMode: true, \ -// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \ +// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; any_format_type_strprintf', \ // RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \ // RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \ // RUN: }}" \ @@ -10,7 +10,7 @@ // RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: { \ -// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \ +// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; any_format_type_strprintf', \ // RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \ // RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \ // RUN: }}" \ @@ -56,12 +56,17 @@ std::string A(const std::string &in) struct S { S(...); }; -std::string bad_format_type_strprintf(const S &, ...); +std::string any_format_type_strprintf(const S &, ...); -std::string unsupported_format_parameter_type() +void unsupported_format_parameter_types() { // 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] + auto s1 = any_format_type_strprintf(L""); + auto s2 = any_format_type_strprintf(42); + + // But if we do pass a character string then that ought to be acceptable. + auto s3 = any_format_type_strprintf("Hello %s", "world"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'fmt::format' instead of 'any_format_type_strprintf' [modernize-use-std-format] + // CHECK-FIXES: auto s3 = fmt::format("Hello {}", "world"); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp index 1a241e3712210..bf56f15c4d632 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp @@ -16,8 +16,8 @@ namespace absl { -template <typename S, typename... Args> -std::string StrFormat(const S &format, const Args&... args); +template <typename... Args> +std::string StrFormat(const std::string &format, const Args&... args); } // namespace absl template <typename T> diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp index 83fbd2e7500c5..ce274ddfe95c4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp @@ -9,15 +9,16 @@ #include <cstdio> #include <string.h> +#include <string> namespace absl { // Use const char * for the format since the real type is hard to mock up. template <typename... Args> -int PrintF(const char *format, const Args&... args); +int PrintF(const std::string_view &format, const Args&... args); template <typename... Args> -int FPrintF(FILE* output, const char *format, const Args&... args); +int FPrintF(FILE* output, const std::string_view &format, const Args&... args); } void printf_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 687b8c0780b01..2c6a651b679d6 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; fmt::printf', \ -// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; bad_format_type_fprintf; fmt::fprintf' \ +// RUN: modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2; any_format_type_printf; fmt::printf', \ +// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; any_format_type_fprintf; fmt::fprintf' \ // RUN: } \ // RUN: }" \ // RUN: -- -isystem %clang_tidy_headers @@ -98,18 +98,25 @@ void wide_string_not_supported() { struct S { S(...) {} }; -int bad_format_type_printf(const S &, ...); -int bad_format_type_fprintf(FILE *, const S &, ...); +int any_format_type_printf(const S &, ...); +int any_format_type_fprintf(FILE *, const S &, ...); 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] + any_format_type_printf(L"Hello %s", "world"); + any_format_type_fprintf(stderr, L"Hello %s", "world"); + any_format_type_printf(42); + any_format_type_fprintf(stderr, 42L); + + // But if we do pass a character string then that ought to be acceptable. + any_format_type_printf("Hello %s\n", "world"); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'any_format_type_printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("Hello {}", "world"); + any_format_type_fprintf(stderr, "Hello %s\n", "world"); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'any_format_type_fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "Hello {}", "world"); } namespace fmt { `````````` </details> https://github.com/llvm/llvm-project/pull/142312 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
