diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -16,6 +16,13 @@ namespace clang { namespace tidy { namespace readability { + +constexpr llvm::StringLiteral ContainerExprName = "container-expr"; +constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr"; +constexpr llvm::StringLiteral AddrOfContainerExprName = + "addr-of-container-expr"; +constexpr llvm::StringLiteral AddressOfName = "address-of"; + ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} @@ -38,69 +45,63 @@ const auto Container = qualType(anyOf(NonTemplateContainerType, TemplateContainerType)); + const auto ContainerExpr = anyOf( + unaryOperator( + hasOperatorName("*"), + hasUnaryOperand( + expr(hasType(pointsTo(Container))).bind(DerefContainerExprName))) + .bind(ContainerExprName), + unaryOperator(hasOperatorName("&"), + hasUnaryOperand(expr(anyOf(hasType(Container), + hasType(references(Container)))) + .bind(AddrOfContainerExprName))) + .bind(ContainerExprName), + expr(anyOf(hasType(Container), hasType(pointsTo(Container)), + hasType(references(Container)))) + .bind(ContainerExprName)); + + const auto Zero = integerLiteral(equals(0)); + + const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]"))); + Finder->addMatcher( unaryOperator( unless(isExpansionInSystemHeader()), hasOperatorName("&"), - hasUnaryOperand(anyOf( - ignoringParenImpCasts( - cxxOperatorCallExpr( - callee(cxxMethodDecl(hasName("operator[]")) - .bind("operator[]")), - argumentCountIs(2), - hasArgument( - 0, - anyOf(ignoringParenImpCasts( - declRefExpr( - to(varDecl(anyOf( - hasType(Container), - hasType(references(Container)))))) - .bind("var")), - ignoringParenImpCasts(hasDescendant( - declRefExpr( - to(varDecl(anyOf( - hasType(Container), - hasType(pointsTo(Container)), - hasType(references(Container)))))) - .bind("var"))))), - hasArgument(1, - ignoringParenImpCasts( - integerLiteral(equals(0)).bind("zero")))) - .bind("operator-call")), - ignoringParenImpCasts( - cxxMemberCallExpr( - hasDescendant( - declRefExpr(to(varDecl(anyOf( - hasType(Container), - hasType(references(Container)))))) - .bind("var")), - argumentCountIs(1), - hasArgument(0, - ignoringParenImpCasts( - integerLiteral(equals(0)).bind("zero")))) - .bind("member-call")), - ignoringParenImpCasts( - arraySubscriptExpr( - hasLHS(ignoringParenImpCasts( - declRefExpr(to(varDecl(anyOf( - hasType(Container), - hasType(references(Container)))))) - .bind("var"))), - hasRHS(ignoringParenImpCasts( - integerLiteral(equals(0)).bind("zero")))) - .bind("array-subscript"))))) - .bind("address-of"), + hasUnaryOperand(expr( + anyOf(cxxOperatorCallExpr(SubscriptOperator, argumentCountIs(2), + hasArgument(0, ContainerExpr), + hasArgument(1, Zero)), + cxxMemberCallExpr(SubscriptOperator, on(ContainerExpr), + argumentCountIs(1), hasArgument(0, Zero)), + arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero)))))) + .bind(AddressOfName), this); } void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { - const auto *UO = Result.Nodes.getNodeAs("address-of"); - const auto *DRE = Result.Nodes.getNodeAs("var"); - - std::string ReplacementText; - ReplacementText = std::string(Lexer::getSourceText( - CharSourceRange::getTokenRange(DRE->getSourceRange()), - *Result.SourceManager, getLangOpts())); - if (DRE->getType()->isPointerType()) + const auto *UO = Result.Nodes.getNodeAs(AddressOfName); + const auto *CE = Result.Nodes.getNodeAs(ContainerExprName); + const auto *DCE = Result.Nodes.getNodeAs(DerefContainerExprName); + const auto *ACE = Result.Nodes.getNodeAs(AddrOfContainerExprName); + + if (!UO || !CE) + return; + + if (DCE && !CE->getType()->isPointerType()) + CE = DCE; + else if (ACE) + CE = ACE; + + SourceRange SrcRange = CE->getSourceRange(); + + std::string ReplacementText{ + Lexer::getSourceText(CharSourceRange::getTokenRange(SrcRange), + *Result.SourceManager, getLangOpts())}; + + if (!isa(CE)) + ReplacementText = "(" + ReplacementText + ")"; + + if (CE->getType()->isPointerType()) ReplacementText += "->data()"; else ReplacementText += ".data()"; 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 --- 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 @@ -109,3 +109,38 @@ // CHECK-MESSAGES: :[[@LINE-1]]:10: 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: {{^ }}return v.data();{{$}} } + +template +struct container_without_data { + using size_type = size_t; + T &operator[](size_type); + const T &operator[](size_type) const; +}; + +template +const T *n(const container_without_data &c) { + // c has no "data" member function, so there should not be a warning here: + return &c[0]; +} + +const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) { + return &v[idx1][idx2][0]; + // CHECK-MESSAGES: :[[@LINE-1]]:10: 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: {{^ }}return v[idx1][idx2].data();{{$}} +} + +std::vector &select(std::vector &u, std::vector &v) { + return v; +} + +int *p(std::vector &v1, std::vector &v2) { + return &select(*&v1, v2)[0]; + // CHECK-MESSAGES: :[[@LINE-1]]:10: 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: {{^ }}return select(*&v1, v2).data();{{$}} +} + +int *q(std::vector ***v) { + return &(***v)[0]; + // CHECK-MESSAGES: :[[@LINE-1]]:10: 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: {{^ }}return (**v)->data();{{$}} +}