Thank you for the review!

- I have modified the doxygen documentation as you recommended.
- The actual language options are used.
- No warnings should be generated for files that compiled without C++11 support 
(although I am not sure that this method works for headers in general).
- Fixed the warning string and removed the redundant empty statement.
- Do not try to fix code that is the result of a macro expansion (I am not sure 
that my solution is idiomatic).
- Added pointer support.
- Fixed some style issues.


http://reviews.llvm.org/D7087

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/ShrinkToFitCheck.cpp
  clang-tidy/readability/ShrinkToFitCheck.h
  test/clang-tidy/readability-shrink-to-fit.cpp

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -8,6 +8,7 @@
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantSmartptrGet.cpp
+  ShrinkToFitCheck.cpp
 
   LINK_LIBS
   clangAST
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ 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 @@
         "readability-function-size");
     CheckFactories.registerCheck<RedundantSmartptrGet>(
         "readability-redundant-smartptr-get");
+    CheckFactories.registerCheck<ShrinkToFitCheck>(
+        "readability-shrink-to-fit");
   }
 };
 
Index: clang-tidy/readability/ShrinkToFitCheck.cpp
===================================================================
--- clang-tidy/readability/ShrinkToFitCheck.cpp
+++ clang-tidy/readability/ShrinkToFitCheck.cpp
@@ -0,0 +1,107 @@
+//===--- 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 CopyCtorCall = constructExpr(hasArgument(
+      0, anyOf(memberExpr(member(valueDecl().bind("CtorParam"))),
+               declRefExpr(hasDeclaration(valueDecl().bind("CtorParam"))),
+               unaryOperator(
+                   has((memberExpr(member(valueDecl().bind("CtorParam")))))),
+               unaryOperator(has((declRefExpr(
+                   hasDeclaration(valueDecl().bind("CtorParam")))))))));
+  const auto SwapParam = expr(anyOf(
+      memberExpr(member(equalsBoundNode("CtorParam"))),
+      declRefExpr(hasDeclaration(equalsBoundNode("CtorParam"))),
+      unaryOperator(has((memberExpr(member(equalsBoundNode("CtorParam")))))),
+      unaryOperator(
+          has((declRefExpr(hasDeclaration(equalsBoundNode("CtorParam"))))))));
+
+  Finder->addMatcher(
+      memberCallExpr(on(hasType(namedDecl(stlShrinkableContainer()))),
+                     callee(methodDecl(hasName("swap"))),
+                     has(memberExpr(hasDescendant(CopyCtorCall))),
+                     hasArgument(0, SwapParam.bind("ContainerToShrink")))
+          .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<CXXMemberCallExpr>("CopyAndSwapTrick");
+  const auto *Container = Result.Nodes.getNodeAs<Expr>("ContainerToShrink");
+  FixItHint Hint;
+
+  if (!MemberCall->getLocStart().isMacroID()) {
+    std::string ReplacementText;
+    if (const auto *UnaryOp = llvm::dyn_cast<UnaryOperator>(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
Index: clang-tidy/readability/ShrinkToFitCheck.h
===================================================================
--- clang-tidy/readability/ShrinkToFitCheck.h
+++ 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
Index: test/clang-tidy/readability-shrink-to-fit.cpp
===================================================================
--- test/clang-tidy/readability-shrink-to-fit.cpp
+++ test/clang-tidy/readability-shrink-to-fit.cpp
@@ -0,0 +1,60 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-shrink-to-fit %t
+// REQUIRES: shell
+
+namespace std {
+template <typename T> struct vector { void swap(vector &other); };
+}
+
+void f() {
+  std::vector<int> v;
+
+  std::vector<int>(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<int> &vref = v;
+  std::vector<int>(vref).swap(vref);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should
+  // CHECK-FIXES: {{^  }}vref.shrink_to_fit();{{$}}
+
+  std::vector<int> *vptr = &v;
+  std::vector<int>(*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<int> v;
+  void f() {
+    std::vector<int>(v).swap(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: the shrink_to_fit method should
+    // CHECK-FIXES: {{^    }}v.shrink_to_fit();{{$}}
+
+    std::vector<int> *vptr = &v;
+    std::vector<int>(*vptr).swap(*vptr);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: the shrink_to_fit method should
+    // CHECK-FIXES: {{^    }}vptr->shrink_to_fit();{{$}}
+  }
+};
+
+template <typename T> void g() {
+  std::vector<int> v;
+
+  std::vector<int>(v).swap(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the shrink_to_fit method should 
+  // CHECK-FIXES: {{^  }}v.shrink_to_fit();{{$}}
+}
+
+#define COPY_AND_SWAP_INT_VEC(x) std::vector<int>(x).swap(x)
+// CHECK-FIXES: #define COPY_AND_SWAP_INT_VEC(x) std::vector<int>(x).swap(x)
+
+void h() {
+  g<int>();
+  g<double>();
+  g<bool>();
+  std::vector<int> 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);{{$}}
+}
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to