shuaiwang updated this revision to Diff 143207. shuaiwang marked 7 inline comments as done. shuaiwang added a comment.
Addressed more comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantDataCallCheck.cpp clang-tidy/readability/RedundantDataCallCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-data-call.rst test/clang-tidy/readability-redundant-data-call.cpp
Index: test/clang-tidy/readability-redundant-data-call.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-data-call.cpp @@ -0,0 +1,108 @@ +// RUN: %check_clang_tidy %s readability-redundant-data-call %t \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: readability-redundant-data-call.Types, \ +// RUN: value: '::std::basic_string;::std::basic_string_view;MyVector'}]}" -- + +namespace std { + +template <class T> +class basic_string { + public: + using size_type = unsigned; + using value_type = T; + using reference = value_type&; + using const_reference = const value_type&; + + reference operator[](size_type); + const_reference operator[](size_type) const; + T* data(); + const T* data() const; +}; + +using string = basic_string<char>; + +template <class T> +class basic_string_view { + public: + using size_type = unsigned; + using const_reference = const T&; + using const_pointer = const T*; + + constexpr const_reference operator[](size_type) const; + constexpr const_pointer data() const noexcept; +}; + +using string_view = basic_string_view<char>; + +} + +template <class T> +class MyVector { + public: + using size_type = unsigned; + using const_reference = const T&; + using const_pointer = const T*; + + const_reference operator[](size_type) const; + const T* data() const noexcept; +}; + +#define DO(x) do { x; } while (false) +#define ACCESS(x) (x) +#define GET(x, i) (x).data()[i] + +template <class T> +class Foo { + public: + char bar(int i) { + return x.data()[i]; + } + private: + T x; +}; + +void f(int i) { + MyVector<int> v; + int x = v.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant call to .data() [readability-redundant-data-call] + // CHECK-FIXES: int x = v[i]; + + std::string s; + char c1 = s.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant call to .data() + // CHECK-FIXES: char c1 = s[i]; + + std::string_view sv; + char c2 = sv.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call to .data() + // CHECK-FIXES: char c2 = sv[i]; + + std::string* ps = &s; + char c3 = ps->data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant call to .data() + // CHECK-FIXES: char c3 = (*ps)[i]; + + char c4 = (*ps).data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant call to .data() + // CHECK-FIXES: char c4 = (*ps)[i]; + + DO(char c5 = s.data()[i]); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant call to .data() + // CHECK-FIXES: DO(char c5 = s[i]); + + char c6 = ACCESS(s).data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: redundant call to .data() + // CHECK-FIXES: char c6 = ACCESS(s)[i]; + + char c7 = ACCESS(s.data())[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant call to .data() + // CHECK-FIXES: char c7 = ACCESS(s)[i]; + + char c8 = ACCESS(s.data()[i]); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant call to .data() + // CHECK-FIXES: char c8 = ACCESS(s[i]); + + char c9 = GET(s, i); + + char c10 = Foo<std::string>{}.bar(i); +} Index: docs/clang-tidy/checks/readability-redundant-data-call.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-data-call.rst @@ -0,0 +1,23 @@ +.. title:: clang-tidy - readability-redundant-data-call + +readability-redundant-data-call +=============================== + +This check finds and suggests removing redundant ``.data()`` calls. Currently +this covers calling ``.data()`` and immediately doing array subscript operation +to obtain a single element, in which case simply calling ``operator[]`` suffice. + +Examples: + +.. code-block:: c++ + + std::string s = ...; + char c = s.data()[i]; // char c = s[i]; + +Options +------- + +.. option:: Types + + The list of type(s) that triggers this check. Default is + `::std::basic_string;::std::basic_string_view;::std::vector;::std::array` Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -218,6 +218,7 @@ readability-named-parameter readability-non-const-parameter readability-redundant-control-flow + readability-redundant-data-call readability-redundant-declaration readability-redundant-function-ptr-dereference readability-redundant-member-init Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -141,6 +141,11 @@ Warns or suggests alternatives if SIMD intrinsics are used which can be replaced by ``std::experimental::simd`` operations. +- New :doc:`readability-redundant-data-call + <clang-tidy/checks/readability-redundant-data-call>` check + + Finds and suggests removing redundant ``.data()`` calls. + - New :doc:`zircon-temporary-objects <clang-tidy/checks/zircon-temporary-objects>` check Index: clang-tidy/readability/RedundantDataCallCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantDataCallCheck.h @@ -0,0 +1,38 @@ +//===--- RedundantDataCallCheck.h - clang-tidy-------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTDATACALLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTDATACALLCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Finds and suggests removing redundant .data() calls. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-data-call.html +class RedundantDataCallCheck : public ClangTidyCheck { +public: + RedundantDataCallCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap& Opts) override; + +private: + const std::vector<std::string> Types; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTDATACALLCHECK_H Index: clang-tidy/readability/RedundantDataCallCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantDataCallCheck.cpp @@ -0,0 +1,72 @@ +//===--- RedundantDataCallCheck.cpp - clang-tidy---------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantDataCallCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +static const char kDefaultTypes[] = + "::std::basic_string;::std::basic_string_view;::std::vector;::std::array"; + +RedundantDataCallCheck::RedundantDataCallCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), Types(utils::options::parseStringList( + Options.get("Types", kDefaultTypes))) { +} + +void RedundantDataCallCheck::registerMatchers(MatchFinder *Finder) { + using namespace ast_matchers; + + const auto TypesMatcher = hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl(hasAnyName( + llvm::SmallVector<StringRef, 8>(Types.begin(), Types.end())))))); + + Finder->addMatcher( + arraySubscriptExpr(hasBase(ignoringParenImpCasts( + cxxMemberCallExpr( + has(memberExpr().bind("member")), + on(hasType(qualType( + unless(anyOf(substTemplateTypeParmType(), + hasDescendant(substTemplateTypeParmType()))), + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))), + callee(namedDecl(hasName("data")))) + .bind("call")))), + this); +} + +void RedundantDataCallCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call"); + if (Result.Context->getSourceManager().isMacroBodyExpansion( + Call->getExprLoc())) { + return; + } + const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member"); + auto DiagBuilder = diag(Member->getMemberLoc(), "redundant call to .data()"); + if (Member->isArrow()) { + DiagBuilder << FixItHint::CreateInsertion(Member->getLocStart(), "(*") + << FixItHint::CreateInsertion(Member->getOperatorLoc(), ")"); + } + DiagBuilder << FixItHint::CreateRemoval( + {Member->getOperatorLoc(), Call->getLocEnd()}); +} + +void RedundantDataCallCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Types", utils::options::serializeStringList(Types)); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -25,6 +25,7 @@ #include "NamedParameterCheck.h" #include "NonConstParameterCheck.h" #include "RedundantControlFlowCheck.h" +#include "RedundantDataCallCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" #include "RedundantMemberInitCheck.h" @@ -68,6 +69,8 @@ "readability-misleading-indentation"); CheckFactories.registerCheck<MisplacedArrayIndexCheck>( "readability-misplaced-array-index"); + CheckFactories.registerCheck<RedundantDataCallCheck>( + "readability-redundant-data-call"); CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>( "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck<RedundantMemberInitCheck>( Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -18,6 +18,7 @@ NonConstParameterCheck.cpp ReadabilityTidyModule.cpp RedundantControlFlowCheck.cpp + RedundantDataCallCheck.cpp RedundantDeclarationCheck.cpp RedundantFunctionPtrDereferenceCheck.cpp RedundantMemberInitCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits