mikecrowe updated this revision to Diff 504475.
mikecrowe added a comment.
Rebase.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145885/new/
https://reviews.llvm.org/D145885
Files:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
@@ -0,0 +1,124 @@
+// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-cstr.StringParameterFunctions, \
+// RUN: value: '::fmt::format; ::fmt::print; ::BaseLogger::operator(); ::BaseLogger::Log'}] \
+// RUN: }" \
+// RUN: -- -isystem %clang_tidy_headers
+#include <string>
+
+namespace fmt {
+ inline namespace v8 {
+ template<typename ...Args>
+ void print(const char *, Args &&...);
+ template<typename ...Args>
+ std::string format(const char *, Args &&...);
+ }
+}
+
+namespace notfmt {
+ inline namespace v8 {
+ template<typename ...Args>
+ void print(const char *, Args &&...);
+ template<typename ...Args>
+ std::string format(const char *, Args &&...);
+ }
+}
+
+void fmt_print(const std::string &s1, const std::string &s2, const std::string &s3) {
+ fmt::print("One:{}\n", s1.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}fmt::print("One:{}\n", s1);
+
+ fmt::print("One:{} Two:{} Three:{}\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:58: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}fmt::print("One:{} Two:{} Three:{}\n", s1, s2, s3);
+}
+
+// There's no c_str() call here, so it shouldn't be touched
+void fmt_print_no_cstr(const std::string &s1, const std::string &s2) {
+ fmt::print("One: {}, Two: {}\n", s1, s2);
+}
+
+// This isn't fmt::print, so it shouldn't be fixed.
+void not_fmt_print(const std::string &s1) {
+ notfmt::print("One: {}\n", s1.c_str());
+}
+
+void fmt_format(const std::string &s1, const std::string &s2, const std::string &s3) {
+ auto r1 = fmt::format("One:{}\n", s1.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}auto r1 = fmt::format("One:{}\n", s1);
+
+ auto r2 = fmt::format("One:{} Two:{} Three:{}\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:53: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:69: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}auto r2 = fmt::format("One:{} Two:{} Three:{}\n", s1, s2, s3);
+}
+
+// There's are c_str() calls here, so it shouldn't be touched
+void fmt_format_no_cstr(const std::string &s1, const std::string &s2) {
+ fmt::format("One: {}, Two: {}\n", s1, s2);
+}
+
+// This is not fmt::format, so it shouldn't be fixed
+std::string not_fmt_format(const std::string &s1) {
+ return notfmt::format("One: {}\n", s1.c_str());
+}
+
+class BaseLogger {
+public:
+ template <typename... Args>
+ void operator()(const char *fmt, Args &&...args) {
+ }
+
+ template <typename... Args>
+ void Log(const char *fmt, Args &&...args) {
+ }
+};
+
+class DerivedLogger : public BaseLogger {};
+class DoubleDerivedLogger : public DerivedLogger {};
+typedef DerivedLogger TypedefDerivedLogger;
+
+void logger1(const std::string &s1, const std::string &s2, const std::string &s3) {
+ BaseLogger LOGGER;
+
+ LOGGER("%s\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER("%s\n", s1, s2, s3);
+
+ DerivedLogger LOGGER2;
+ LOGGER2("%d %s\n", 42, s1.c_str(), s2.c_str(), s3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER2("%d %s\n", 42, s1, s2, s3);
+
+ DoubleDerivedLogger LOGGERD;
+ LOGGERD("%d %s\n", 42, s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGERD("%d %s\n", 42, s1, s2, s3);
+
+ TypedefDerivedLogger LOGGERT;
+ LOGGERT("%d %s\n", 42, s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGERT("%d %s\n", 42, s1, s2, s3);
+}
+
+void logger2(const std::string &s1, const std::string &s2) {
+ BaseLogger LOGGER3;
+
+ LOGGER3.Log("%s\n", s1.c_str(), s2.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER3.Log("%s\n", s1, s2);
+
+ DerivedLogger LOGGER4;
+ LOGGER4.Log("%d %s\n", 42, s1.c_str(), s2.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER4.Log("%d %s\n", 42, s1, s2);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
@@ -5,3 +5,26 @@
Finds unnecessary calls to ``std::string::c_str()`` and ``std::string::data()``.
+
+Options
+-------
+
+.. option:: StringParameterFunctions
+
+ A semicolon-separated list of function names for which any arguments
+ which are the result of calling ``std::string::c_str()`` or
+ ``std::string::data()`` should be changed to pass the object directly.
+ This is particularly useful for functions that behave like
+ ``std::format`` and ``std::print`` (though arguments to both of those
+ are already detected automatically in C++20 and C++2b mode
+ respectively.) The names may be free functions, member functions, or
+ even operators.
+
+ For example, when using `{fmt} <https://fmt.dev/>`, you might like to
+ use::
+ -config="{CheckOptions: [{key: readability-redundant-string-cstr.StringParameterFunctions, value: '::fmt::format; ::fmt::print'}]}"
+
+ For a logging class named ``Logger`` with a ``Log`` method and an
+ ``operator()`` that passes its arguments to ``std::format`` or
+ ``fmt::format`` you might like to use::
+ -config="{CheckOptions: [{key: readability-redundant-string-cstr.StringParameterFunctions, value: '::Logger::Log;::Logger::operator()'}]}"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,7 +149,9 @@
- 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``.
+ arguments to ``std::print`` and ``std::format``. The same check can be
+ used for functions that behave similarly by using the
+ ``StringParameterFunction`` option.
- Deprecated check-local options `HeaderFileExtensions`
in :doc:`bugprone-dynamic-static-initializers
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
@@ -16,13 +16,15 @@
/// Finds unnecessary calls to `std::string::c_str()`.
class RedundantStringCStrCheck : public ClangTidyCheck {
public:
- RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ std::vector<StringRef> StringParameterFunctions;
};
} // namespace clang::tidy::readability
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -11,6 +11,8 @@
//===----------------------------------------------------------------------===//
#include "RedundantStringCStrCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/FixIt.h"
@@ -69,6 +71,17 @@
} // end namespace
+RedundantStringCStrCheck::RedundantStringCStrCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StringParameterFunctions(utils::options::parseStringList(
+ Options.get("StringParameterFunctions", ""))) {
+ if (getLangOpts().CPlusPlus20)
+ StringParameterFunctions.push_back("::std::format");
+ if (getLangOpts().CPlusPlus2b)
+ StringParameterFunctions.push_back("::std::print");
+}
+
void RedundantStringCStrCheck::registerMatchers(
ast_matchers::MatchFinder *Finder) {
// Match expressions of type 'string' or 'string*'.
@@ -183,15 +196,13 @@
hasArgument(0, StringCStrCallExpr))),
this);
- if (getLangOpts().CPlusPlus20) {
+ if (!StringParameterFunctions.empty()) {
// 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"))),
+ callExpr(callee(functionDecl(matchers::matchesAnyListedName(
+ StringParameterFunctions))),
forEachArgumentWithParam(StringCStrCallExpr,
parmVarDecl()))),
this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
