diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp index 11756d10a8221..5e1cd70b26930 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp @@ -71,20 +71,20 @@ void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) { const auto Zero = integerLiteral(equals(0)); - const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]"))); - - Finder->addMatcher( + const auto AddressOfMatcher = unaryOperator( unless(isExpansionInSystemHeader()), hasOperatorName("&"), - 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); + hasUnaryOperand(ignoringParenImpCasts(expr(anyOf( + cxxOperatorCallExpr( + hasOverloadedOperatorName("[]"), argumentCountIs(2), + hasArgument(0, ContainerExpr), hasArgument(1, Zero)), + cxxMemberCallExpr(callee(cxxMethodDecl(hasName("operator[]"))), + on(ContainerExpr), argumentCountIs(1), + hasArgument(0, Zero)), + arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero))))))) + .bind(AddressOfName); + + Finder->addMatcher(traverse(TK_AsIs, AddressOfMatcher), this); } void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { @@ -101,14 +101,20 @@ void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) { else if (ACE) CE = ACE; - SourceRange SrcRange = CE->getSourceRange(); + const Expr *PrintedCE = CE->IgnoreParenImpCasts(); + + const SourceRange SrcRange = PrintedCE->getSourceRange(); std::string ReplacementText{ Lexer::getSourceText(CharSourceRange::getTokenRange(SrcRange), *Result.SourceManager, getLangOpts())}; - if (!isa(CE)) + const auto *OpCall = dyn_cast(PrintedCE); + const bool NeedsParens = + OpCall ? (OpCall->getOperator() != OO_Subscript) + : !isa( + PrintedCE); + if (NeedsParens) ReplacementText = "(" + ReplacementText + ")"; if (CE->getType()->isPointerType()) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8f4be0d1cb259..11337aa3d628e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -362,7 +362,7 @@ Changes in existing checks - Improved :doc:`misc-const-correctness ` check to avoid false - positives when pointers is transferred to non-const references + positives when pointers is transferred to non-const references and avoid false positives of function pointer and fix false positives on return of non-const pointer. @@ -429,6 +429,10 @@ Changes in existing checks comparisons to ``npos``. Internal changes may cause new rare false positives in non-standard containers. +- Improved :doc:`readability-container-data-pointer + ` check by correctly + adding parentheses when the container expression is a dereference. + - Improved :doc:`readability-container-size-empty ` check by correctly generating fix-it hints when size method is called from implicit ``this``, 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 a8e0eb6d262e6..80a8e03b25def 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 @@ -35,6 +35,12 @@ template struct enable_if { typedef T type; }; + +template +struct unique_ptr { + T &operator*() const; + T *operator->() const; +}; } template @@ -144,3 +150,18 @@ int *r() { // 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 holder.v.data(); } + +void s(std::unique_ptr> p) { + f(&(*p)[0]); + // CHECK-MESSAGES: :[[@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: f((*p).data()); +} + +template +void u(std::unique_ptr p) { + f(&(*p)[0]); + // CHECK-MESSAGES: :[[@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: f((*p).data()); +} + +template void u>(std::unique_ptr>);