[clang-tools-extra] 626586f - Re-Revert "clang-tidy: introduce readability-containter-data-pointer check"
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 14 19:28:57 PDT 2021
Author: Nico Weber
Date: 2021-09-14T22:27:59-04:00
New Revision: 626586fc253c6f032aedb325dba6b1ff3f11875e
URL: https://github.com/llvm/llvm-project/commit/626586fc253c6f032aedb325dba6b1ff3f11875e
DIFF: https://github.com/llvm/llvm-project/commit/626586fc253c6f032aedb325dba6b1ff3f11875e.diff
LOG: Re-Revert "clang-tidy: introduce readability-containter-data-pointer check"
This reverts commit 49992c04148e5327bef9bd2dff53a0d46004b4b4.
The test is still failing on Windows, see comments on https://reviews.llvm.org/D108893
Added:
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
Removed:
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index eba0ab98cb37a..78256d6f73251 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -7,7 +7,6 @@ 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
deleted file mode 100644
index 3a670509ec2e2..0000000000000
--- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ /dev/null
@@ -1,117 +0,0 @@
-//===--- 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
deleted file mode 100644
index 0f0f8233f5d84..0000000000000
--- a/clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ /dev/null
@@ -1,45 +0,0 @@
-//===--- 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 2d6540283ded5..366541a0ed487 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,7 +12,6 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ConstReturnTypeCheck.h"
-#include "ContainerDataPointerCheck.h"
#include "ContainerSizeEmptyCheck.h"
#include "ConvertMemberFunctionsToStatic.h"
#include "DeleteNullPointerCheck.h"
@@ -63,8 +62,6 @@ class ReadabilityModule : public ClangTidyModule {
"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>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1b1f00de4d69c..609064f7610b0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,11 +91,6 @@ New checks
variables and function parameters only.
-- New :doc:`readability-data-pointer <clang-tidy/checks/readability-data-pointer` check.
-
- Finds cases where code could use ``data()`` rather than the address of the
- element at index 0 in a container.
-
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
deleted file mode 100644
index 46febd26496d6..0000000000000
--- a/clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
+++ /dev/null
@@ -1,13 +0,0 @@
-.. title:: clang-tidy - readability-data-pointer
-
-readability-data-pointer
-========================
-
-Finds cases where code could use ``data()`` rather than the address of the
-element at index 0 in a container. This pattern is commonly used to materialize
-a pointer to the backing data of a container. ``std::vector`` and
-``std::string`` provide a ``data()`` accessor to retrieve the data pointer which
-should be preferred.
-
-This also ensures that in the case that the container is empty, the data pointer
-access does not perform an errant memory access.
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
deleted file mode 100644
index f217b60d52537..0000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ /dev/null
@@ -1,110 +0,0 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t
-
-typedef __SIZE_TYPE__ size_t;
-
-namespace std {
-template <typename T>
-struct vector {
- using size_type = size_t;
-
- vector();
- explicit vector(size_type);
-
- T *data();
- const T *data() const;
-
- T &operator[](size_type);
- const T &operator[](size_type) const;
-};
-
-template <typename T>
-struct basic_string {
- using size_type = size_t;
-
- basic_string();
-
- T *data();
- const T *data() const;
-
- T &operator[](size_t);
- const T &operator[](size_type) const;
-};
-
-typedef basic_string<char> string;
-typedef basic_string<wchar_t> wstring;
-
-template <typename T>
-struct is_integral;
-
-template <>
-struct is_integral<size_t> {
- static const bool value = true;
-};
-
-template <bool, typename T = void>
-struct enable_if { };
-
-template <typename T>
-struct enable_if<true, T> {
- typedef T type;
-};
-}
-
-template <typename T>
-void f(const T *);
-
-#define z (0)
-
-void g(size_t s) {
- std::vector<unsigned char> b(s);
- f(&((b)[(z)]));
- // 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(b.data());{{$}}
-}
-
-void h() {
- std::string s;
- f(&((s).operator[]((z))));
- // 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(s.data());{{$}}
-
- std::wstring w;
- f(&((&(w))->operator[]((z))));
- // 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(w.data());{{$}}
-}
-
-template <typename T, typename U,
- typename = typename std::enable_if<std::is_integral<U>::value>::type>
-void i(U s) {
- std::vector<T> b(s);
- f(&b[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(b.data());{{$}}
-}
-
-template void i<unsigned char, size_t>(size_t);
-
-void j(std::vector<unsigned char> * const v) {
- f(&(*v)[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(v->data());{{$}}
-}
-
-void k(const std::vector<unsigned char> &v) {
- f(&v[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(v.data());{{$}}
-}
-
-void l() {
- unsigned char b[32];
- f(&b[0]);
- // CHECK-MESSAGES-NOT: :[[@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]
-}
-
-template <typename T>
-void m(const std::vector<T> &v) {
- const T *p = &v[0];
- // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-}
More information about the cfe-commits
mailing list