From e94b7c24c809dd398a146786e9612afa99ed0395 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Fri, 23 Jan 2015 15:10:37 +0000 Subject: [PATCH] [clang-tidy] Use shrink_to_fit instead of copy and swap trick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shrink_to_fit() method is more readable and more effective than the copy and swap trick to reduce the capacity of a shrinkable container. Note that, the shrink_to_fit() method is only available in C++11 and up. Example: std::vector(v).swap(v); will be replaced with v.shrink_to_fit(); http://reviews.llvm.org/D7087 Patch by Gábor Horváth! llvm-svn: 226912 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../readability/ShrinkToFitCheck.cpp | 109 ++++++++++++++++++ .../clang-tidy/readability/ShrinkToFitCheck.h | 35 ++++++ .../clang-tidy/readability-shrink-to-fit.cpp | 75 ++++++++++++ 5 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.h create mode 100644 clang-tools-extra/test/clang-tidy/readability-shrink-to-fit.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index f0944c443483..567ac1d80ae8 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_library(clangTidyReadabilityModule NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp RedundantSmartptrGet.cpp + ShrinkToFitCheck.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 315912b128bf..a92f8a4313dc 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -15,6 +15,7 @@ #include "ElseAfterReturnCheck.h" #include "FunctionSize.h" #include "RedundantSmartptrGet.h" +#include "ShrinkToFitCheck.h" namespace clang { namespace tidy { @@ -33,6 +34,8 @@ public: "readability-function-size"); CheckFactories.registerCheck( "readability-redundant-smartptr-get"); + CheckFactories.registerCheck( + "readability-shrink-to-fit"); } }; diff --git a/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.cpp b/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.cpp new file mode 100644 index 000000000000..1c9e0a8117ae --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.cpp @@ -0,0 +1,109 @@ +//===--- ShrinkToFitCheck.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 "ShrinkToFitCheck.h" + +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace { +bool isShrinkableContainer(llvm::StringRef ClassName) { + static const llvm::StringSet<> Shrinkables = [] { + llvm::StringSet<> RetVal; + RetVal.insert("std::deque"); + RetVal.insert("std::basic_string"); + RetVal.insert("std::vector"); + return RetVal; + }(); + return Shrinkables.find(ClassName) != Shrinkables.end(); +} +} + +namespace clang { +namespace ast_matchers { +AST_MATCHER(NamedDecl, stlShrinkableContainer) { + return isShrinkableContainer(Node.getQualifiedNameAsString()); +} +} // namespace ast_matchesr +} // namespace clang + +namespace clang { +namespace tidy { + +void ShrinkToFitCheck::registerMatchers(MatchFinder *Finder) { + // Swap as a function need not to be considered, because rvalue can not + // be bound to a non-const reference. + const auto ShrinkableAsMember = + memberExpr(member(valueDecl().bind("ContainerDecl"))); + const auto ShrinkableAsDecl = + declRefExpr(hasDeclaration(valueDecl().bind("ContainerDecl"))); + const auto CopyCtorCall = constructExpr( + hasArgument(0, anyOf(ShrinkableAsMember, ShrinkableAsDecl, + unaryOperator(has(ShrinkableAsMember)), + unaryOperator(has(ShrinkableAsDecl))))); + const auto SwapParam = expr(anyOf( + memberExpr(member(equalsBoundNode("ContainerDecl"))), + declRefExpr(hasDeclaration(equalsBoundNode("ContainerDecl"))), + unaryOperator(has(memberExpr(member(equalsBoundNode("ContainerDecl"))))), + unaryOperator( + has(declRefExpr(hasDeclaration(equalsBoundNode("ContainerDecl"))))))); + + Finder->addMatcher( + memberCallExpr(on(hasType(namedDecl(stlShrinkableContainer()))), + callee(methodDecl(hasName("swap"))), + has(memberExpr(hasDescendant(CopyCtorCall))), + hasArgument(0, SwapParam.bind("ContainerToShrink")), + unless(isInTemplateInstantiation())) + .bind("CopyAndSwapTrick"), + this); +} + +void ShrinkToFitCheck::check(const MatchFinder::MatchResult &Result) { + const LangOptions &Opts = Result.Context->getLangOpts(); + + if (!Opts.CPlusPlus11) + return; + + const auto *MemberCall = + Result.Nodes.getNodeAs("CopyAndSwapTrick"); + const auto *Container = Result.Nodes.getNodeAs("ContainerToShrink"); + FixItHint Hint; + + if (!MemberCall->getLocStart().isMacroID()) { + std::string ReplacementText; + if (const auto *UnaryOp = llvm::dyn_cast(Container)) { + ReplacementText = + Lexer::getSourceText(CharSourceRange::getTokenRange( + UnaryOp->getSubExpr()->getSourceRange()), + *Result.SourceManager, Opts); + ReplacementText += "->shrink_to_fit()"; + } else { + ReplacementText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Container->getSourceRange()), + *Result.SourceManager, Opts); + ReplacementText += ".shrink_to_fit()"; + } + + Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(), + ReplacementText); + } + + diag(MemberCall->getLocStart(), "the shrink_to_fit method should be used " + "to reduce the capacity of a shrinkable " + "container") + << Hint; +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.h b/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.h new file mode 100644 index 000000000000..04fb6dd23bd8 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ShrinkToFitCheck.h @@ -0,0 +1,35 @@ +//===--- ShrinkToFitCheck.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_SHRINK_TO_FIT_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SHRINK_TO_FIT_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Replace copy and swap tricks on shrinkable containers with the +/// \c shrink_to_fit() method call. +/// +/// The \c shrink_to_fit() method is more readable and more effective than +/// the copy and swap trick to reduce the capacity of a shrinkable container. +/// Note that, the \c shrink_to_fit() method is only available in C++11 and up. +class ShrinkToFitCheck : public ClangTidyCheck { +public: + ShrinkToFitCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SHRINK_TO_FIT_H diff --git a/clang-tools-extra/test/clang-tidy/readability-shrink-to-fit.cpp b/clang-tools-extra/test/clang-tidy/readability-shrink-to-fit.cpp new file mode 100644 index 000000000000..039d95f1aa93 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/readability-shrink-to-fit.cpp @@ -0,0 +1,75 @@ +// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-shrink-to-fit %t +// REQUIRES: shell + +namespace std { +template struct vector { void swap(vector &other); }; +} + +void f() { + std::vector v; + + std::vector(v).swap(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should be used to reduce the capacity of a shrinkable container [readability-shrink-to-fit] + // CHECK-FIXES: {{^ }}v.shrink_to_fit();{{$}} + + std::vector &vref = v; + std::vector(vref).swap(vref); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}vref.shrink_to_fit();{{$}} + + std::vector *vptr = &v; + std::vector(*vptr).swap(*vptr); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}vptr->shrink_to_fit();{{$}} +} + +struct X { + std::vector v; + void f() { + std::vector(v).swap(v); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}v.shrink_to_fit();{{$}} + + std::vector *vptr = &v; + std::vector(*vptr).swap(*vptr); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}vptr->shrink_to_fit();{{$}} + } +}; + +template void g() { + std::vector v; + std::vector(v).swap(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}v.shrink_to_fit();{{$}} + + std::vector v2; + std::vector(v2).swap(v2); + // CHECK-FIXES: {{^ }}std::vector(v2).swap(v2);{{$}} +} + +template void g2() { + std::vector v; + std::vector(v).swap(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}v.shrink_to_fit();{{$}} + + T v3; + T(v3).swap(v3); + // CHECK-FIXES: {{^ }}T(v3).swap(v3);{{$}} +} + +#define COPY_AND_SWAP_INT_VEC(x) std::vector(x).swap(x) +// CHECK-FIXES: #define COPY_AND_SWAP_INT_VEC(x) std::vector(x).swap(x) + +void h() { + g(); + g(); + g(); + g2>(); + std::vector v; + COPY_AND_SWAP_INT_VEC(v); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should + // CHECK-FIXES: {{^ }}COPY_AND_SWAP_INT_VEC(v);{{$}} +} +