[clang-tidy] Fix readability-container-data-pointer check

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<DeclRefExpr, ArraySubscriptExpr, CXXOperatorCallExpr, CallExpr,
-           MemberExpr>(CE))
+  const auto *OpCall = dyn_cast<CXXOperatorCallExpr>(PrintedCE);
+  const bool NeedsParens =
+      OpCall ? (OpCall->getOperator() != OO_Subscript)
+             : !isa<DeclRefExpr, MemberExpr, ArraySubscriptExpr, CallExpr>(
+                   PrintedCE);
+  if (NeedsParens)
     ReplacementText = "(" + ReplacementText + ")";
 
   if (CE->getType()->isPointerType())

clang-tools-extra/docs/ReleaseNotes.rst

@@ -362,7 +362,7 @@ Changes in existing checks
 
 - Improved :doc:`misc-const-correctness
   <clang-tidy/checks/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
+  <clang-tidy/checks/readability/container-data-pointer>` check by correctly
+  adding parentheses when the container expression is a dereference.
+
 - Improved :doc:`readability-container-size-empty
   <clang-tidy/checks/readability/container-size-empty>` check by correctly
   generating fix-it hints when size method is called from implicit ``this``,

clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

@@ -35,6 +35,12 @@ template <typename T>
 struct enable_if<true, T> {
   typedef T type;
 };
+
+template <typename T>
+struct unique_ptr {
+  T &operator*() const;
+  T *operator->() const;
+};
 }
 
 template <typename T>
@@ -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<std::vector<unsigned char>> 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 <typename T>
+void u(std::unique_ptr<T> 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::vector<int>>(std::unique_ptr<std::vector<int>>);