acoster created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
acoster requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Suggests replacing the call with `std::string::size`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140968
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp
clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst
clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy -check-suffix=0 %s readability-strlen-string-cstr %t -- -config="{CheckOptions: [{key: readability-strlen-string-cstr.EnableForDataMethod, value: false}]}"
+// RUN: %check_clang_tidy -check-suffix=1 %s readability-strlen-string-cstr %t -- -config="{CheckOptions: [{key: readability-strlen-string-cstr.EnableForDataMethod, value: true}]}"
+int strlen(const char *);
+int strnlen(const char *, int);
+int strnlen_s(const char *, int);
+
+int wcslen(const wchar_t *);
+int wcsnlen_s(const wchar_t *, int);
+
+namespace std {
+template <typename T> class allocator {};
+template <typename T> class char_traits {};
+
+template <typename C, typename T = std::char_traits<C>,
+ typename A = std::allocator<C>>
+class basic_string {
+public:
+ basic_string();
+ basic_string(const C *, unsigned int size);
+ const C *c_str() const;
+ const C *data() const;
+ int size() const;
+};
+
+using wstring = basic_string<wchar_t>;
+using string = basic_string<char>;
+
+int strlen(const char *);
+int wcslen(const wchar_t *);
+} // namespace std
+
+void f1() {
+ std::string str1("a", 1);
+ int length = strlen(str1.c_str());
+ // CHECK-MESSAGES-0: [[@LINE-1]]:16: warning: redundant call to 'strlen' {{.*}}
+ // CHECK-FIXES-0: {{^ }}int length = str1.size();{{$}}
+ // CHECK-MESSAGES-1: [[@LINE-3]]:16: warning: redundant call to 'strlen' {{.*}}
+ // CHECK-FIXES-1: {{^ }}int length = str1.size();{{$}}
+
+ length = strnlen(str1.data(), 30);
+ // CHECK-MESSAGES-1: [[@LINE-1]]:12: warning: redundant call to 'strnlen' {{.*}}
+ // CHECK-FIXES-1: {{^ }}length = str1.size();{{$}}
+
+ const std::string *p1 = &str1;
+ length = std::strlen(p1->c_str()) + 30;
+ // CHECK-MESSAGES-0: [[@LINE-1]]:12: warning: redundant call {{.*}}
+ // CHECK-FIXES-0: {{^ }}length = p1->size() + 30;{{$}}
+ // CHECK-MESSAGES-1: [[@LINE-3]]:12: warning: redundant call {{.*}}
+ // CHECK-FIXES-1: {{^ }}length = p1->size() + 30;{{$}}
+}
+
+void f2() {
+ std::wstring wstr1;
+
+ int length = wcslen(wstr1.c_str());
+ // CHECK-MESSAGES-0: [[@LINE-1]]:16: warning: redundant call to 'wcslen' {{.*}}
+ // CHECK-FIXES-0: {{^ }}int length = wstr1.size();{{$}}
+ // CHECK-MESSAGES-1: [[@LINE-3]]:16: warning: redundant call to 'wcslen' {{.*}}
+ // CHECK-FIXES-1: {{^ }}int length = wstr1.size();{{$}}
+}
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-strlen-string-cstr
+
+readability-strlen-string-cstr
+==============================
+
+Finds calls to ``strlen`` and similar functions where the result
+of ``std::basic_string::c_str`` or ``std::basic_string::data`` is used
+as an argument.
+
+Example
+-------
+
+.. code-block:: c++
+
+ std::string str1{"foo"}
+
+ // Use str1.size()
+ size_t length = strlen(str1.c_str());
+
+
+Options
+-------
+
+.. option:: EnableForDataMethod
+
+ Default is `true`.
+
+ When `false`, the check will not warn then the result of
+ ``std::basic_string::data`` is used as an argument for ``strlen``.
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -362,6 +362,7 @@
`readability-static-accessed-through-instance <readability/static-accessed-through-instance.html>`_, "Yes"
`readability-static-definition-in-anonymous-namespace <readability/static-definition-in-anonymous-namespace.html>`_, "Yes"
`readability-string-compare <readability/string-compare.html>`_, "Yes"
+ `readability-strlen-string-cstr <readability/strlen-string-cstr.html>`_, "Yes"
`readability-suspicious-call-argument <readability/suspicious-call-argument.html>`_,
`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release.html>`_, "Yes"
`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix.html>`_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -129,6 +129,12 @@
Warns when `empty()` is used on a range and the result is ignored. Suggests `clear()`
if it is an existing member function.
+- New :doc:`readability-strlen-string-cstr
+ <clang-tidy/checks/readability/strlen-string-cstr>` check.
+
+Warns when the return value of `std::basic_string::c_str` or `std::basic_string::data`
+is used as the argument for `strlen`, and suggests a fix.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h
@@ -0,0 +1,44 @@
+//===--- StrlenStringCStrCheck.h - clang-tidy ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds calls to `strlen(std::string::c_str())`.
+class StrlenStringCStrCheck : public ClangTidyCheck {
+public:
+ StrlenStringCStrCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ EnableForDataMethod(Options.get("EnableForDataMethod", false)) {}
+
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
+ Options.store(Opts, "EnableForDataMethod", EnableForDataMethod);
+ }
+
+ 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:
+ const bool EnableForDataMethod;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H
Index: clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp
@@ -0,0 +1,65 @@
+//===- StrlenStringCStrCheck.cpp - Check for strlen(string::c_str()) calls
+//-----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements a check for calls to `strlen(std::string::c_str())`.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StrlenStringCStrCheck.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void StrlenStringCStrCheck::registerMatchers(
+ ast_matchers::MatchFinder *Finder) {
+ const auto MethodNameMatcher =
+ EnableForDataMethod ? hasAnyName("c_str", "data") : hasAnyName("c_str");
+
+ const auto StrlenFunctions = functionDecl(
+ hasAnyName("::strlen", "::std::strlen", "::strnlen", "::strnlen_s",
+ "::wcslen", "::std::wcslen", "::wcsnlen_s"));
+ const auto CStrCall = cxxMemberCallExpr(callee(
+ cxxMethodDecl(MethodNameMatcher,
+ ofClass(cxxRecordDecl(hasName("::std::basic_string"))))));
+ const auto StrlenCall =
+ callExpr(callee(StrlenFunctions), hasArgument(0, CStrCall.bind("arg")))
+ .bind("strlen");
+ Finder->addMatcher(StrlenCall, this);
+}
+
+void StrlenStringCStrCheck::check(
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ const auto *StrlenCall = Result.Nodes.getNodeAs<CallExpr>("strlen");
+ const auto *Arg = Result.Nodes.getNodeAs<CXXMemberCallExpr>("arg");
+
+ // Remove the `strlen(` (or similar) prefix.
+ CharSourceRange StrlenRange = CharSourceRange::getCharRange(
+ StrlenCall->getBeginLoc(), Arg->getBeginLoc());
+ // Remove the `)` (and additional strnlen arguments, if any) from the strlen
+ // call.
+ CharSourceRange CloseParenRange =
+ CharSourceRange::getCharRange(Arg->getEndLoc(), StrlenCall->getEndLoc());
+
+ const auto *CStrCallee = cast<MemberExpr>(Arg->getCallee());
+ diag(StrlenCall->getBeginLoc(), "redundant call to %0 and %1")
+ << StrlenCall->getDirectCallee() << Arg->getMethodDecl()
+ << FixItHint::CreateRemoval(StrlenRange)
+ << FixItHint::CreateRemoval(CloseParenRange)
+ << FixItHint::CreateReplacement(CStrCallee->getMemberLoc(), "size");
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -47,6 +47,7 @@
#include "StaticAccessedThroughInstanceCheck.h"
#include "StaticDefinitionInAnonymousNamespaceCheck.h"
#include "StringCompareCheck.h"
+#include "StrlenStringCStrCheck.h"
#include "SuspiciousCallArgumentCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
@@ -135,6 +136,8 @@
"readability-redundant-string-init");
CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
"readability-simplify-boolean-expr");
+ CheckFactories.registerCheck<StrlenStringCStrCheck>(
+ "readability-strlen-string-cstr");
CheckFactories.registerCheck<SuspiciousCallArgumentCheck>(
"readability-suspicious-call-argument");
CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -44,6 +44,7 @@
StaticAccessedThroughInstanceCheck.cpp
StaticDefinitionInAnonymousNamespaceCheck.cpp
StringCompareCheck.cpp
+ StrlenStringCStrCheck.cpp
SuspiciousCallArgumentCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
