[PATCH] [Clang Tidy]: Use shrink_to_fit instead of copy and swap trick.

[Alexander Kornienko]

Thanks for contributing the check! The code looks mostly good, but see a few comments inline.

> There was already a checker which targeted C++11, however it did not contain any check, which 

>  standard is used during the analysis. Maybe a check should be added to suppress C++11 specific 

>  warnings when clang tidy is invoked in C++03 mode?


We could bail out in the check() method it the language standard is below C++11. Ideally, standard-specific checks would be able to avoid registering matchers when parsing code not in supported standard mode. However, there's no good way to do this as we don't know which minimum standard version the code is supposed to be compatible with. And at the point where we register matchers, there's no information on the list of files and the used compilation options yet. So looking at the standard in the check() method is the best we can do.


================
Comment at: clang-tidy/readability/ShrinkToFitCheck.cpp:48
@@ +47,3 @@
+  const auto CopyCtorCall = constructExpr(hasArgument(
+      0, anyOf(memberExpr(member(valueDecl().bind("CtorParam"))),
+               declRefExpr(hasDeclaration(valueDecl().bind("CtorParam"))))));
----------------
Maybe also handle pointer-type variables/data members?

================
Comment at: clang-tidy/readability/ShrinkToFitCheck.cpp:71
@@ +70,3 @@
+      CharSourceRange::getTokenRange(ContainerToShrink->getSourceRange()),
+      *Result.SourceManager, LangOptions());
+  ReplacementText += ".shrink_to_fit()";
----------------
Please use Result.Context->getLangOpts() instead (I've fixed the other two checks using LangOptions()).

================
Comment at: clang-tidy/readability/ShrinkToFitCheck.cpp:74
@@ +73,3 @@
+
+  diag(MemberCall->getLocStart(), "The shrink_to_fit method should be used "
+                                  "to reduce the capacity of a shrinkable "
----------------
s/The/the/

================
Comment at: clang-tidy/readability/ShrinkToFitCheck.cpp:79
@@ +78,3 @@
+                                      ReplacementText);
+  ;
+}
----------------
?

================
Comment at: clang-tidy/readability/ShrinkToFitCheck.h:18
@@ +17,3 @@
+
+/// \brief Replace copy and swap tricks on shrinkable containers with the \c
+/// shrink_to_fit() method call.
----------------
nit: I don't know whether Doxygen cares, but from the human point of view, \c would make more sense next to the word it relates to.

================
Comment at: test/clang-tidy/readability-shrink-to-fit.cpp:15
@@ +14,3 @@
+
+  std::vector<int>& vref = v;
+  std::vector<int>(vref).swap(vref);
----------------
nit: Please put the "&" to the variable.

================
Comment at: test/clang-tidy/readability-shrink-to-fit.cpp:39
@@ +38,3 @@
+#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)
+
----------------
I don't know why the fix doesn't get applied here (which would be wrong). I think, the check is avoiding this by pure coincidence. But it may be fine to postpone solving this until someone stumbles upon the problem and has a good test case.

================
Comment at: test/clang-tidy/readability-shrink-to-fit.cpp:47
@@ +46,3 @@
+  COPY_AND_SWAP_INT_VEC(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: The shrink_to_fit method should 
+}
----------------
Please add
  // CHECK-FIXES: {{^  }}COPY_AND_SWAP_INT_VEC(v);{{$}}
to ensure this doesn't get spoiled.

http://reviews.llvm.org/D7087

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/