From: Nico Weber Date: Wed, 15 Sep 2021 02:27:59 +0000 (-0400) Subject: Re-Revert "clang-tidy: introduce readability-containter-data-pointer check" X-Git-Tag: upstream/15.0.7~31465 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=626586fc253c6f032aedb325dba6b1ff3f11875e;p=platform%2Fupstream%2Fllvm.git 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 --- diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index eba0ab9..78256d6 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 3a67050..0000000 --- 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("address-of"); - const auto *DRE = Result.Nodes.getNodeAs("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 0f0f823..0000000 --- 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 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 2d65402..366541a 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 @@ public: "readability-braces-around-statements"); CheckFactories.registerCheck( "readability-const-return-type"); - CheckFactories.registerCheck( - "readability-container-data-pointer"); CheckFactories.registerCheck( "readability-container-size-empty"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1b1f00d..609064f 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 -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 -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 string; -typedef basic_string wstring; - -template -struct is_integral; - -template <> -struct is_integral { - static const bool value = true; -}; - -template -struct enable_if { }; - -template -struct enable_if { - typedef T type; -}; -} - -template -void f(const T *); - -#define z (0) - -void g(size_t s) { - std::vector 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 ::value>::type> -void i(U s) { - std::vector 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(size_t); - -void j(std::vector * 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 &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 -void m(const std::vector &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] -}