https://github.com/neoncube2 updated https://github.com/llvm/llvm-project/pull/71304
>From ab1e2e7487fdc7b1e16e563c63e225a3dd39da05 Mon Sep 17 00:00:00 2001 From: Eli Black <elibla...@hotmail.com> Date: Sun, 5 Nov 2023 13:43:20 +0800 Subject: [PATCH] [clang-tidy] Improve `container-data-pointer` check to use `c_str()` instead of `data()` when it's available. Fixes #55026 --- .../readability/ContainerDataPointerCheck.cpp | 26 ++++++++++++------- .../readability/ContainerDataPointerCheck.h | 4 +-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/container-data-pointer.rst | 8 ++---- .../readability/container-data-pointer.cpp | 12 ++++----- 5 files changed, 30 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp index a05e228520c9ef1..eb76d05667d300a 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -40,8 +40,10 @@ void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) { cxxRecordDecl( unless(matchers::matchesAnyListedName(IgnoredContainers)), isSameOrDerivedFrom( - namedDecl( - has(cxxMethodDecl(isPublic(), hasName("data")).bind("data"))) + namedDecl(anyOf(has(cxxMethodDecl(isPublic(), hasName("c_str")) + .bind("c_str")), + has(cxxMethodDecl(isPublic(), hasName("data")) + .bind("data")))) .bind("container"))) .bind("record"); @@ -93,6 +95,8 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { const auto *DCE = Result.Nodes.getNodeAs<Expr>(DerefContainerExprName); const auto *ACE = Result.Nodes.getNodeAs<Expr>(AddrOfContainerExprName); + const auto *CStrMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("c_str"); + if (!UO || !CE) return; @@ -111,16 +115,18 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { MemberExpr>(CE)) ReplacementText = "(" + ReplacementText + ")"; - if (CE->getType()->isPointerType()) - ReplacementText += "->data()"; - else - ReplacementText += ".data()"; + ReplacementText += CE->getType()->isPointerType() ? "->" : "."; + ReplacementText += CStrMethod ? "c_str()" : "data()"; + + std::string Description = + CStrMethod + ? "'c_str' should be used instead of taking the address of the 0-th " + "element" + : "'data' should be used for accessing the data pointer instead of " + "taking the address of the 0-th element"; FixItHint Hint = FixItHint::CreateReplacement(UO->getSourceRange(), ReplacementText); - diag(UO->getBeginLoc(), - "'data' should be used for accessing the data pointer instead of taking " - "the address of the 0-th element") - << Hint; + diag(UO->getBeginLoc(), Description) << Hint; } } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h index 2a15b95095171f1..f5c1a974ff84801 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h @@ -12,8 +12,8 @@ #include "../ClangTidyCheck.h" namespace clang::tidy::readability { -/// Checks whether a call to `operator[]` and `&` can be replaced with a call to -/// `data()`. +/// Finds cases where code references the address of the element at index 0 in a +/// container and replaces them with calls to ``data()`` or ``c_str()``. /// /// This only replaces the case where the offset being accessed through the /// subscript operation is a known constant 0. This avoids a potential invalid diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ecfb3aa9267f140..6477f2243d31e18 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -238,6 +238,10 @@ Changes in existing checks Casting to ``void`` no longer suppresses issues by default, control this behavior with the new `AllowCastToVoid` option. +- Improved :doc:`container-data-pointer + <clang-tidy/checks/readability/container-data-pointer>` check + to use ``c_str()`` when it's present on a container. + - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check to ignore ``static`` variables declared within the scope of diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst index 0d10829ed3c2f9b..8a6b48c58005bf2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst @@ -3,13 +3,9 @@ readability-container-data-pointer ================================== -Finds cases where code could use ``data()`` rather than the address of the -element at index 0 in a container. This pattern is commonly used to materialize -a pointer to the backing data of a container. ``std::vector`` and -``std::string`` provide a ``data()`` accessor to retrieve the data pointer which -should be preferred. +Finds cases where code references the address of the element at index 0 in a container and replaces them with calls to ``data()`` or ``c_str()``. -This also ensures that in the case that the container is empty, the data pointer +Using ``data()`` or ``c_str()`` is more readable and ensures that if the container is empty, the data pointer access does not perform an errant memory access. Options diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp index 7d8500ed4affe36..a53f303eba1f5d3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp @@ -52,15 +52,15 @@ void g(size_t s) { void h() { std::string s; f(&((s).operator[]((z)))); - // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] - // CHECK-FIXES-CLASSIC: {{^ }}f(s.data());{{$}} - // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer] + // CHECK-FIXES-CLASSIC: {{^ }}f(s.c_str());{{$}} + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer] std::wstring w; f(&((&(w))->operator[]((z)))); - // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] - // CHECK-FIXES-CLASSIC: {{^ }}f(w.data());{{$}} - // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer] + // CHECK-FIXES-CLASSIC: {{^ }}f(w.c_str());{{$}} + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'c_str' should be used instead of taking the address of the 0-th element [readability-container-data-pointer] } template <typename T, typename U, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits