[clang-tools-extra] 57b78fa - [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 12 14:43:20 PDT 2023
Author: Mike Crowe
Date: 2023-03-12T21:42:53Z
New Revision: 57b78faa9eb08017d95bb4e43242648953b08c13
URL: https://github.com/llvm/llvm-project/commit/57b78faa9eb08017d95bb4e43242648953b08c13
DIFF: https://github.com/llvm/llvm-project/commit/57b78faa9eb08017d95bb4e43242648953b08c13.diff
LOG: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr
std::format (C++20) and std::print (C++23) are perfectly happy to accept
std::string arguments. Converting them to C-style strings by calling
c_str() is unnecessary and may cause extra walking of the string to
determine its length.
Depends on D144216
Reviewed By: carlosgalvezp, PiotrZSL
Differential Revision: https://reviews.llvm.org/D143342
Added:
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-format.cpp
Modified:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index 1b21f70f0bf64..d1fdf8341f710 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -182,6 +182,20 @@ void RedundantStringCStrCheck::registerMatchers(
// 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
index c0b2d118ec4b6..6d48ff7bf7eb1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -146,6 +146,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`readability-redundant-string-cstr
+ <clang-tidy/checks/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
index e9d31b929c6c4..80b41da61c2e3 100644
--- 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 @@ typedef basic_string<char32> u32string;
template <typename C, typename T = char_traits<C>>
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<char> string_view;
typedef basic_string_view<wchar_t> 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
index 0000000000000..e58c8d8b69c05
--- /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 <string>
+
+namespace std {
+ template<typename T>
+ struct type_identity { using type = T; };
+ template<typename T>
+ using type_identity_t = typename type_identity<T>::type;
+
+ template <typename CharT, typename... Args>
+ struct basic_format_string {
+ consteval basic_format_string(const CharT *format) : str(format) {}
+ basic_string_view<CharT, std::char_traits<CharT>> str;
+ };
+
+ template<typename... Args>
+ using format_string = basic_format_string<char, type_identity_t<Args>...>;
+
+ template<typename... Args>
+ using wformat_string = basic_format_string<wchar_t, type_identity_t<Args>...>;
+
+#if defined(TEST_STDFORMAT)
+ template<typename ...Args>
+ std::string format(format_string<Args...>, Args &&...);
+ template<typename ...Args>
+ std::string format(wformat_string<Args...>, Args &&...);
+#endif // TEST_STDFORMAT
+
+#if defined(TEST_STDPRINT)
+ template<typename ...Args>
+ void print(format_string<Args...>, Args &&...);
+ template<typename ...Args>
+ void print(wformat_string<Args...>, Args &&...);
+#endif // TEST_STDPRINT
+}
+
+namespace notstd {
+#if defined(TEST_STDFORMAT)
+ template<typename ...Args>
+ std::string format(const char *, Args &&...);
+ template<typename ...Args>
+ std::string format(const wchar_t *, Args &&...);
+#endif // TEST_STDFORMAT
+#if defined(TEST_STDPRINT)
+ template<typename ...Args>
+ void print(const char *, Args &&...);
+ template<typename ...Args>
+ 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<typename ...Args>
+std::string format(const char *, Args &&...);
+template<typename ...Args>
+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<typename ...Args>
+void print(const char *, Args &&...);
+template<typename ...Args>
+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
More information about the cfe-commits
mailing list