[clang-tools-extra] [clang-tidy] Enhance container-data-pointer-check to suggest `c_str` for consts (PR #190590)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 01:41:19 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Gaurav Dhingra (gxyd)
<details>
<summary>Changes</summary>
## Description
Fixes #<!-- -->55026
I took inspiration from earlier closed PR: https://github.com/llvm/llvm-project/pull/71304, which guided me to a good solution here.
With an aim to get my first PR into the llvm-project, kindly let me know if this needs more work.
---
Full diff: https://github.com/llvm/llvm-project/pull/190590.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp (+56-11)
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp (+26)
``````````diff
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
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,
``````````
</details>
https://github.com/llvm/llvm-project/pull/190590
More information about the cfe-commits
mailing list