[PATCH] D17491: Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:41
@@ +40,3 @@
+                       Function->parameters().begin();
+  if (Index >= Function->getNumParams()) {
+    return;
----------------
Please add a comment about when this happens (template parameter packs? C-style variadic functions?).

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:46
@@ +45,3 @@
+      Param->getType().getCanonicalType().isConstQualified();
+  // Do not trigger on non-const value parameters when:
+  // 1. they are in a constructor definition since they can likely trigger
----------------
Please add an empty line before the comment.

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:48
@@ +47,3 @@
+  // 1. they are in a constructor definition since they can likely trigger
+  // misc-move-constructor-init which will suggest to move the argument.
+  // 2. they are not only used as const.
----------------
nit: Add three spaces before the first word for better alignment.

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:58
@@ +57,3 @@
+                              "copied for each invocation; consider "
+                              "making this a reference"
+                            : "the parameter '%0' is copied for each "
----------------
s/making this a reference/making it a reference/?

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:62
@@ +61,3 @@
+                              "consider making it a const reference")
+      << Param->getName();
+  // Do not propose fixes in macros since we cannot place them correctly.
----------------
What if parameter doesn't have a name? Should we print an index ("parameter #n is copied ...").

================
Comment at: docs/clang-tidy/checks/performance-unnecessary-value-param.rst:6
@@ +5,3 @@
+
+Flags value parameter declarations of expensive to copy types that are copied
+for each invocation but it would suffice to pass them by const reference.
----------------
Add an example?

================
Comment at: docs/clang-tidy/checks/performance-unnecessary-value-param.rst:16
@@ +15,3 @@
+
+1. the parameter is const qualified.
+2. the parameter is not const, but only const methods or operators are invoked
----------------
nit: a `;` seems to be more suitable as a trailing punctuation here.


Repository:
  rL LLVM

http://reviews.llvm.org/D17491