[PATCH] D18408: readability check for const params in declarations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Looks good in general. A few nits.


================
Comment at: clang-tidy/readability/AvoidConstParamsInDecls.cpp:22
@@ +21,3 @@
+
+SourceRange GetTypeRange(const ParmVarDecl &Param) {
+  if (Param.getIdentifier() != nullptr)
----------------
"Function names ... should be camel case, and start with a lower case letter (e.g. openFile() or isFoo())."
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

================
Comment at: clang-tidy/readability/AvoidConstParamsInDecls.cpp:50
@@ +49,3 @@
+
+  auto DiagBuilder = diag(Param->getLocStart(),
+                          "parameter %0 is const-qualified in the function "
----------------
nit: The `Diag` name is more common for this purpose.

================
Comment at: clang-tidy/readability/AvoidConstParamsInDecls.cpp:51-53
@@ +50,5 @@
+  auto DiagBuilder = diag(Param->getLocStart(),
+                          "parameter %0 is const-qualified in the function "
+                          "declaration; const-qualification of parameters "
+                          "only has an effect in function definitions");
+  if (Param->getName().empty()) {
----------------
This is the best wording I could come up with in an internal review, but it's still not ideal. If someone knows how to improve this, ideas are welcome.

================
Comment at: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst:4
@@ +3,3 @@
+readability-avoid-const-params-in-decls
+==============================================
+
----------------
To many '='s.

================
Comment at: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst:8
@@ +7,3 @@
+
+`const` values in declarations do not have any affect on the signature of a
+function, so they should not be put there.  For example:
----------------
Either "do not have any effect on ..." or "do not affect ..."

================
Comment at: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst:15
@@ +14,3 @@
+
+  void f(const string);   // bad. const is top level
+  void f(const string&);  // ok. const is not top level
----------------
Please use proper capitalization and punctuation in the comments.


http://reviews.llvm.org/D18408