https://github.com/gxyd updated https://github.com/llvm/llvm-project/pull/190590
>From cbe1f957b5d3b0f06ae27e2b84641f71f5f8d91d Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Sun, 5 Apr 2026 17:15:17 +0530 Subject: [PATCH 1/5] [clang-tidy] Enhance container-data-pointer-check to suggest c_ptr for consts Improves the container-data-pointer readability check to suggest use of 'c_ptr' instead of 'data' Fixes #55026 --- .../readability/ContainerDataPointerCheck.cpp | 67 ++++++++++++++++--- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp index 6dfef3aa247ef..9d304fd2a8478 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -38,8 +38,9 @@ void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) { cxxRecordDecl( unless(matchers::matchesAnyListedRegexName(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("container"))) .bind("record"); @@ -91,6 +92,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 *CStrDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("c_str"); + if (!UO || !CE) return; @@ -99,6 +102,46 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { else if (ACE) CE = ACE; + SourceRange ReplacementRange = UO->getSourceRange(); + bool UseCStr = false; + if (CStrDecl) { + auto Parents = Result.Context->getParents(*UO); + + if (!Parents.empty()) { + if (const auto *VD = Parents[0].get<VarDecl>()) { + QualType VarType = VD->getType(); + if (VarType->isPointerType()) { + QualType PointeeType = VarType->getPointeeType(); + UseCStr = PointeeType.isConstQualified(); + } + } else if (const auto *ICE = Parents[0].get<ImplicitCastExpr>()) { + QualType CastType = ICE->getType(); + if (CastType->isPointerType()) { + QualType PointeeType = CastType->getPointeeType(); + UseCStr = PointeeType.isConstQualified(); + } + } else if (const auto *Cast = Parents[0].get<CastExpr>()) { + QualType CastType = Cast->getType(); + if (CastType->isPointerType()) { + QualType PointeeType = CastType->getPointeeType(); + UseCStr = PointeeType.isConstQualified(); + if (UseCStr) { + // if it's a const cast, use the Cast range as replacement range + // e.g. (const char*)&s[0] -> s.c_str() + ReplacementRange = Cast->getSourceRange(); + } + } + } + } + + if (!UseCStr) { + QualType ContainerType = CE->getType(); + if (ContainerType->isPointerType()) + ContainerType = ContainerType->getPointeeType(); + UseCStr = ContainerType.isConstQualified(); + } + } + const SourceRange SrcRange = CE->getSourceRange(); std::string ReplacementText{ @@ -112,16 +155,18 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { if (NeedsParens) ReplacementText = "(" + ReplacementText + ")"; - if (CE->getType()->isPointerType()) - ReplacementText += "->data()"; - else - ReplacementText += ".data()"; + ReplacementText += CE->getType()->isPointerType() ? "->" : "."; + ReplacementText += UseCStr ? "c_str()" : "data()"; + llvm::StringRef Description = UseCStr + ? "'c_str' should be used for accessing " + "the data pointer 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"; const 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; + FixItHint::CreateReplacement(ReplacementRange, ReplacementText); + diag(UO->getBeginLoc(), Description) << Hint; } } // namespace clang::tidy::readability >From 8e0456e3680f8eb0e9411dac7d5aaad5f552d6f7 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Mon, 6 Apr 2026 14:00:12 +0530 Subject: [PATCH 2/5] add test cases for 'c_ptr' usage instead of 'data' the following cases are tested: - char* p1 = &s[0]; - const char* p2 = &s[0]; - f((const char*)&((s).operator[]((z)))); - f(&((&(cs))->operator[]((z)))); // where `cs` is constant - const char* p3 = &cs[0]; // where `cs` is constant --- .../readability/container-data-pointer.cpp | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) 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 4fd228a554d7d..3efa7ed4501bd 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 @@ -25,11 +25,37 @@ void h() { // 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] + char* p1 = &s[0]; + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:14: 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: char* p1 = s.data(); + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:14: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] + + const char* p2 = &s[0]; + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:20: warning: 'c_str' 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: const char* p2 = s.c_str(); + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:20: warning: 'c_str' should be used for accessing the data pointer 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] + + f((const char*)&((s).operator[]((z)))); + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:18: warning: 'c_str' 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.c_str()); + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:18: warning: 'c_str' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] + + const std::string cs; + f(&((&(cs))->operator[]((z)))); + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'c_str' 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(cs.c_str()); + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'c_str' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] + + const char* p3 = &cs[0]; + // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:20: warning: 'c_str' 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: const char* p3 = cs.c_str(); + // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:20: warning: 'c_str' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] } template <typename T, typename U, >From 730237beade99aa4392e98b99762140cb2bd98b0 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Mon, 6 Apr 2026 16:03:01 +0530 Subject: [PATCH 3/5] [clang-tidy] update docs & and add ReleaseNotes entry update documentation for readability check container-data-pointer and add a ReleaseNotes entry --- .../readability/ContainerDataPointerCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/container-data-pointer.rst | 26 ++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h index 71fde87fbb093..7cb9fd6a2ea68 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::readability { /// Checks whether a call to `operator[]` and `&` can be replaced with a call to -/// `data()`. +/// `data()` or (for consts, when available) `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 36e311341f336..a48786ed6c0d9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -275,6 +275,10 @@ Changes in existing checks - Add support for annotation of user-defined types as having the same moved-from semantics as standard smart pointers. +- Improved :doc:`container-data-pointer + <clang-tidy/checks/readability/container-data-pointer>` check + to use ``c_str()`` for const's when available and present on a container. + - Improved :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check by adding the `AllowExplicitObjectParameters` option. When enabled, 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 4b2ef6ef1fbea..3c9f13c90661f 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,15 +3,35 @@ 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 +Finds cases where code could use ``data()`` or ``c_str()`` 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. +This check suggests ``data()`` for non-const pointer contexts and ``c_str()`` +for const pointer contexts when available. This provides better semantic +clarity: ``c_str()`` explicitly indicates read-only access to string data, +while ``data()`` may allow modifications. + This also ensures that in the case that the container is empty, the data pointer access does not perform an errant memory access. +Examples +-------- + +.. code-block: c++ + + std::string s; + std::vector<int> v; + + char* p1 = &s[0]; // Warning: use s.data() + const char* p2 = &s[0] // Warning: use s.c_str() + int* p3 = &v[0]; // Warning: use v.data() + + const std::string cs; + const char* p4 = &cs[0]; // Warning: use cs.c_str() + Options ------- >From 176635ebd8c0dfabbf9406bd1b253378ab7acb18 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Mon, 6 Apr 2026 16:37:59 +0530 Subject: [PATCH 4/5] use '%select' for llvm::StringRef instead of ternary operator --- .../readability/ContainerDataPointerCheck.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp index 9d304fd2a8478..1fdb54e652b55 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -158,15 +158,11 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { ReplacementText += CE->getType()->isPointerType() ? "->" : "."; ReplacementText += UseCStr ? "c_str()" : "data()"; - llvm::StringRef Description = UseCStr - ? "'c_str' should be used for accessing " - "the data pointer 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"; const FixItHint Hint = FixItHint::CreateReplacement(ReplacementRange, ReplacementText); - diag(UO->getBeginLoc(), Description) << Hint; + diag(UO->getBeginLoc(), + "'%select{data|c_str}0' should be used for accessing the data pointer " + "instead of taking the address of the 0-th element") + << UseCStr << Hint; } } // namespace clang::tidy::readability >From 291976bea9bc9787c43e68c0577bd3c47d51ce87 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Mon, 6 Apr 2026 16:43:19 +0530 Subject: [PATCH 5/5] remove redundant cast removal step when improving readability --- .../readability/ContainerDataPointerCheck.cpp | 14 +------------- .../readability/container-data-pointer.cpp | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp index 1fdb54e652b55..427924aaddcc2 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -102,7 +102,6 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { else if (ACE) CE = ACE; - SourceRange ReplacementRange = UO->getSourceRange(); bool UseCStr = false; if (CStrDecl) { auto Parents = Result.Context->getParents(*UO); @@ -114,22 +113,11 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { QualType PointeeType = VarType->getPointeeType(); UseCStr = PointeeType.isConstQualified(); } - } else if (const auto *ICE = Parents[0].get<ImplicitCastExpr>()) { - QualType CastType = ICE->getType(); - if (CastType->isPointerType()) { - QualType PointeeType = CastType->getPointeeType(); - UseCStr = PointeeType.isConstQualified(); - } } else if (const auto *Cast = Parents[0].get<CastExpr>()) { QualType CastType = Cast->getType(); if (CastType->isPointerType()) { QualType PointeeType = CastType->getPointeeType(); UseCStr = PointeeType.isConstQualified(); - if (UseCStr) { - // if it's a const cast, use the Cast range as replacement range - // e.g. (const char*)&s[0] -> s.c_str() - ReplacementRange = Cast->getSourceRange(); - } } } } @@ -159,7 +147,7 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { ReplacementText += UseCStr ? "c_str()" : "data()"; const FixItHint Hint = - FixItHint::CreateReplacement(ReplacementRange, ReplacementText); + FixItHint::CreateReplacement(UO->getSourceRange(), ReplacementText); diag(UO->getBeginLoc(), "'%select{data|c_str}0' should be used for accessing the data pointer " "instead of taking the address of the 0-th element") 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 3efa7ed4501bd..78f3dc4eeb277 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 @@ -43,7 +43,7 @@ void h() { f((const char*)&((s).operator[]((z)))); // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:18: warning: 'c_str' 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.c_str()); + // CHECK-FIXES-CLASSIC: f((const char*)s.c_str()); // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:18: warning: 'c_str' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer] const std::string cs; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
