[PATCH] Add clang-tidy check to remove redundant .get() calls on smart pointers.

Alexander Kornienko alexfh at google.com
Wed Mar 26 10:53:39 PDT 2014


  Thanks for working on this!

  Looks good in general, a few comments in-line. Could you also run the check on llvm and post the summary of results?


================
Comment at: clang-tidy/misc/RedundantSmartptrGet.cpp:56
@@ +55,3 @@
+void RedundantSmartptrGet::check(const MatchFinder::MatchResult &Result) {
+  {
+    // First verify that the types match.
----------------
What's the reason to enclose this in a compound statement?
If you want to limit visibility of local variables, then it's better to make this a separate function.

================
Comment at: clang-tidy/misc/RedundantSmartptrGet.cpp:75
@@ +74,3 @@
+
+  const std::string SmartptrText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
----------------
Why not use StringRef here?

================
Comment at: clang-tidy/misc/RedundantSmartptrGet.cpp:80
@@ +79,3 @@
+  const std::string Replacement =
+      IsPtrToPtr ? (Twine("*") + SmartptrText).str() : SmartptrText;
+  diag(GetCall->getLocStart(), "Redundant get() call on smart pointer.")
----------------
nit: How about Twine(IsPtrToPtr ? "*" : "", SmartptrText).str()? It's bit shorter and seems not to be less clear.

================
Comment at: test/clang-tidy/redundant-smartptr-get.cpp:44
@@ +43,3 @@
+  std::unique_ptr<Bar>().get()->Do();
+  // CHECK: warning: Redundant get() call on smart pointer. [misc-redundant-smartptr-get]
+
----------------
Maybe add line and column numbers to make the tests more precise?
// CHECK: :[[@LINE-1]]:<column number>: warning: ...

I'd also leave the whole error message only once, and abbreviate all other occasions so that they don't exceed the column limit and thus are easier to read.


http://llvm-reviews.chandlerc.com/D3186



More information about the cfe-commits mailing list