diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -182,6 +182,20 @@ // directly. hasArgument(0, StringCStrCallExpr))), this); + + if (getLangOpts().CPlusPlus20) { + // Detect redundant 'c_str()' calls in parameters passed to std::format in + // C++20 onwards and std::print in C++23 onwards. + Finder->addMatcher( + traverse(TK_AsIs, + callExpr(callee(functionDecl( + getLangOpts().CPlusPlus2b + ? hasAnyName("::std::print", "::std::format") + : hasName("::std::format"))), + forEachArgumentWithParam(StringCStrCallExpr, + parmVarDecl()))), + this); + } } void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -146,6 +146,10 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`readability-redundant-string-cstr + ` check to recognise + unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in + arguments to ``std::print`` and ``std::format``. - Deprecated check-local options `HeaderFileExtensions` in :doc:`bugprone-dynamic-static-initializers diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -62,7 +62,8 @@ template > struct basic_string_view { - basic_string_view(const C* s); + const C *str; + constexpr basic_string_view(const C* s) : str(s) {} }; typedef basic_string_view string_view; typedef basic_string_view wstring_view; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-format.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-format.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-format.cpp @@ -0,0 +1,214 @@ +// RUN: %check_clang_tidy -check-suffix=STDFORMAT -std=c++20 %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers -DTEST_STDFORMAT +// RUN: %check_clang_tidy -check-suffixes=STDFORMAT,STDPRINT -std=c++2b %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers -DTEST_STDFORMAT -DTEST_STDPRINT +#include + +namespace std { + template + struct type_identity { using type = T; }; + template + using type_identity_t = typename type_identity::type; + + template + struct basic_format_string { + consteval basic_format_string(const CharT *format) : str(format) {} + basic_string_view> str; + }; + + template + using format_string = basic_format_string...>; + + template + using wformat_string = basic_format_string...>; + +#if defined(TEST_STDFORMAT) + template + std::string format(format_string, Args &&...); + template + std::string format(wformat_string, Args &&...); +#endif // TEST_STDFORMAT + +#if defined(TEST_STDPRINT) + template + void print(format_string, Args &&...); + template + void print(wformat_string, Args &&...); +#endif // TEST_STDPRINT +} + +namespace notstd { +#if defined(TEST_STDFORMAT) + template + std::string format(const char *, Args &&...); + template + std::string format(const wchar_t *, Args &&...); +#endif // TEST_STDFORMAT +#if defined(TEST_STDPRINT) + template + void print(const char *, Args &&...); + template + void print(const wchar_t *, Args &&...); +#endif // TEST_STDPRINT +} + +std::string return_temporary(); +std::wstring return_wtemporary(); + +#if defined(TEST_STDFORMAT) +void std_format(const std::string &s1, const std::string &s2, const std::string &s3) { + auto r1 = std::format("One:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:37: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r1 = std::format("One:{}\n", s1); + + auto r2 = std::format("One:{} Two:{} Three:{} Four:{}\n", s1.c_str(), s2, s3.c_str(), return_temporary().c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:61: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-2]]:77: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-3]]:89: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r2 = std::format("One:{} Two:{} Three:{} Four:{}\n", s1, s2, s3, return_temporary()); + + using namespace std; + auto r3 = format("Four:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:33: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r3 = format("Four:{}\n", s1); +} + +void std_format_wide(const std::wstring &s1, const std::wstring &s2, const std::wstring &s3) { + auto r1 = std::format(L"One:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:38: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r1 = std::format(L"One:{}\n", s1); + + auto r2 = std::format(L"One:{} Two:{} Three:{} Four:{}\n", s1.c_str(), s2, s3.c_str(), return_wtemporary().c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:62: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-2]]:78: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-3]]:90: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r2 = std::format(L"One:{} Two:{} Three:{} Four:{}\n", s1, s2, s3, return_wtemporary()); + + using namespace std; + auto r3 = format(L"Four:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDFORMAT: :[[@LINE-1]]:34: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDFORMAT: {{^ }}auto r3 = format(L"Four:{}\n", s1); +} + +// There's are c_str() calls here, so it shouldn't be touched. +std::string std_format_no_cstr(const std::string &s1, const std::string &s2) { + return std::format("One: {}, Two: {}\n", s1, s2); +} + +// There's are c_str() calls here, so it shouldn't be touched. +std::string std_format_no_cstr_wide(const std::string &s1, const std::string &s2) { + return std::format(L"One: {}, Two: {}\n", s1, s2); +} + +// This is not std::format, so it shouldn't be fixed. +std::string not_std_format(const std::string &s1) { + return notstd::format("One: {}\n", s1.c_str()); + + using namespace notstd; + format("One: {}\n", s1.c_str()); +} + +// This is not std::format, so it shouldn't be fixed. +std::string not_std_format_wide(const std::string &s1) { + return notstd::format(L"One: {}\n", s1.c_str()); + + using namespace notstd; + format(L"One: {}\n", s1.c_str()); +} +#endif // TEST_STDFORMAT + +#if defined(TEST_STDPRINT) +void std_print(const std::string &s1, const std::string &s2, const std::string &s3) { + std::print("One:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}std::print("One:{}\n", s1); + + std::print("One:{} Two:{} Three:{} Four:{}\n", s1.c_str(), s2, s3.c_str(), return_temporary().c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:50: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDPRINT: :[[@LINE-2]]:66: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDPRINT: :[[@LINE-3]]:78: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}std::print("One:{} Two:{} Three:{} Four:{}\n", s1, s2, s3, return_temporary()); + + using namespace std; + print("Four:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:22: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}print("Four:{}\n", s1); +} + +void std_print_wide(const std::wstring &s1, const std::wstring &s2, const std::wstring &s3) { + std::print(L"One:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:27: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}std::print(L"One:{}\n", s1); + + std::print(L"One:{} Two:{} Three:{} Four:{}\n", s1.c_str(), s2, s3.c_str(), return_wtemporary().c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:51: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDPRINT: :[[@LINE-2]]:67: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-MESSAGES-STDPRINT: :[[@LINE-3]]:79: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}std::print(L"One:{} Two:{} Three:{} Four:{}\n", s1, s2, s3, return_wtemporary()); + + using namespace std; + print(L"Four:{}\n", s1.c_str()); + // CHECK-MESSAGES-STDPRINT: :[[@LINE-1]]:23: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES-STDPRINT: {{^ }}print(L"Four:{}\n", s1); +} + +// There's no c_str() call here, so it shouldn't be touched. +void std_print_no_cstr(const std::string &s1, const std::string &s2) { + std::print("One: {}, Two: {}\n", s1, s2); +} + +// There's no c_str() call here, so it shouldn't be touched. +void std_print_no_cstr_wide(const std::wstring &s1, const std::wstring &s2) { + std::print(L"One: {}, Two: {}\n", s1, s2); +} + +// This isn't std::print, so it shouldn't be fixed. +void not_std_print(const std::string &s1) { + notstd::print("One: {}\n", s1.c_str()); + + using namespace notstd; + print("One: {}\n", s1.c_str()); +} + +// This isn't std::print, so it shouldn't be fixed. +void not_std_print_wide(const std::string &s1) { + notstd::print(L"One: {}\n", s1.c_str()); + + using namespace notstd; + print(L"One: {}\n", s1.c_str()); +} +#endif // TEST_STDPRINT + +#if defined(TEST_STDFORMAT) +// We can't declare these earlier since they make the "using namespace std" +// tests ambiguous. +template +std::string format(const char *, Args &&...); +template +std::string format(const wchar_t *, Args &&...); + +// This is not std::format, so it shouldn't be fixed. +std::string not_std_format2(const std::wstring &s1) { + return format("One: {}\n", s1.c_str()); +} + +// This is not std::format, so it shouldn't be fixed. +std::string not_std_format2_wide(const std::wstring &s1) { + return format(L"One: {}\n", s1.c_str()); +} +#endif // TEST_STDFORMAT + +#if defined(TEST_STDPRINT) +template +void print(const char *, Args &&...); +template +void print(const wchar_t *, Args &&...); + +// This isn't std::print, so it shouldn't be fixed. +void not_std_print2(const std::string &s1) { + print("One: {}\n", s1.c_str()); +} + +// This isn't std::print, so it shouldn't be fixed. +void not_std_print2_wide(const std::string &s1) { + print(L"One: {}\n", s1.c_str()); +} +#endif // TEST_STDPRINT