[clang-tools-extra] [clang-tidy] Fix `readability-container-data-pointer` check (PR #165636)

[Baranov Victor via cfe-commits]

================
@@ -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);
----------------
vbvictor wrote:

Thank you for detailed description! Your English is good enough to understand everything:)

So based on the feedback, I think we should drop support for `template unique_ptr<T>` case with `TK_AsIs` because it may introduce the following problem:

Consider 2 specializations, one with `MyArray` which doesn't have `data()` member:
```cpp
template <typename T>
void u(std::unique_ptr<T> p) {
  f(&(*p)[0]); // warning because of 'std::vector<int>'
}

template void u<std::vector<int>>(std::unique_ptr<std::vector<int>>);
template void u<MyArray>(std::unique_ptr<MyArray>); // don't have `.data()`
```

So the check will fire for `std::vector` but we can't actually change `[0]` to `data()` because our other type `MyArray` will give compilation error because it doesn't have `data()` member.

This is I think why `TK_IgnoreUnlessSpelledInSource` was used in the first place: to avoid such false-positives.

I guess we can not just revert to previous version and LGTM

https://github.com/llvm/llvm-project/pull/165636