[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