[PATCH] D18149: Add check for unneeded copies of locals

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Thank you for the patch! Looks mostly good, a few style nits.


================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:21
@@ -20,2 +20,3 @@
 using namespace ::clang::ast_matchers;
+using clang::tidy::decl_ref_expr_utils::isOnlyUsedAsConst;
 
----------------
No need for clang::tidy:: here.

================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:43
@@ +42,3 @@
+  auto localVarCopiedFrom = [](
+      const ast_matchers::internal::Matcher<Expr>& CopyCtorArg) {
+    return compoundStmt(
----------------
I guess, there's no need for ast_matchers:: here.

================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:127
@@ +126,3 @@
+  auto Diagnostic = diag(NewVar.getLocation(),
+                         "local copy %0 of the variable %1 is never modified, "
+                         "consider avoiding the copy")
----------------
nit: I'd use a semicolon instead of the comma here, like in other clang-tidy messages. 

================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.h:29
@@ -28,3 +28,3 @@
 public:
-  UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context)
+  UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext* Context)
       : ClangTidyCheck(Name, Context) {}
----------------
LLVM style is to put *'s and &'s to the variable. Looks like your clang-format didn't understand it should use LLVM style. You might want to explicitly specify `-style=llvm` next time.


http://reviews.llvm.org/D18149