Revert "Revert "clang-tidy: introduce readability-containter-data-pointer check""

This reverts commit 76dc8ac36d07cebe8cfe8fe757323562bb36df94.

Restore the change.  The test had an incorrect negative from testing.
The test is expected to trigger a failure as mentioned in the review
comments.  This corrects the test and should resolve the failure.

4 files changed, 166 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 78256d6..eba0ab9 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ConstReturnTypeCheck.cpp
+  ContainerDataPointerCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ConvertMemberFunctionsToStatic.cpp
   DeleteNullPointerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
new file mode 100644
index 0000000..3a67050
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -0,0 +1,117 @@
+//===--- ContainerDataPointerCheck.cpp - clang-tidy -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ContainerDataPointerCheck.h"
+
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/StringRef.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
+  const auto Record =
+      cxxRecordDecl(
+          isSameOrDerivedFrom(
+              namedDecl(
+                  has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))
+                  .bind("container")))
+          .bind("record");
+
+  const auto NonTemplateContainerType =
+      qualType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(Record))));
+  const auto TemplateContainerType =
+      qualType(hasUnqualifiedDesugaredType(templateSpecializationType(
+          hasDeclaration(classTemplateDecl(has(Record))))));
+
+  const auto Container =
+      qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
+
+  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"),
+      this);
+}
+
+void ContainerDataPointerCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *UO = Result.Nodes.getNodeAs<UnaryOperator>("address-of");
+  const auto *DRE = Result.Nodes.getNodeAs<DeclRefExpr>("var");
+
+  std::string ReplacementText;
+  ReplacementText = std::string(Lexer::getSourceText(
+      CharSourceRange::getTokenRange(DRE->getSourceRange()),
+      *Result.SourceManager, getLangOpts()));
+  if (DRE->getType()->isPointerType())
+    ReplacementText += "->data()";
+  else
+    ReplacementText += ".data()";
+
+  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;
+}
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
new file mode 100644
index 0000000..0f0f823
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -0,0 +1,45 @@
+//===--- ContainerDataPointerCheck.h - clang-tidy ---------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERDATAPOINTERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERDATAPOINTERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+/// Checks whether a call to `operator[]` and `&` can be replaced with a call to
+/// `data()`.
+///
+/// This only replaces the case where the offset being accessed through the
+/// subscript operation is a known constant 0.  This avoids a potential invalid
+/// memory access when the container is empty.  Cases where the constant is not
+/// explictly zero can be addressed through the clang static analyzer, and those
+/// which cannot be statically identified can be caught using UBSan.
+class ContainerDataPointerCheck : public ClangTidyCheck {
+public:
+  ContainerDataPointerCheck(StringRef Name, ClangTidyContext *Context);
+
+  bool isLanguageVersionSupported(const LangOptions &LO) const override {
+    return LO.CPlusPlus11;
+  }
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERDATAPOINTERCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 366541a..2d65402 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ConstReturnTypeCheck.h"
+#include "ContainerDataPointerCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ConvertMemberFunctionsToStatic.h"
 #include "DeleteNullPointerCheck.h"
@@ -62,6 +63,8 @@ public:
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ConstReturnTypeCheck>(
         "readability-const-return-type");
+    CheckFactories.registerCheck<ContainerDataPointerCheck>(
+        "readability-container-data-pointer");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ConvertMemberFunctionsToStatic>(