[PATCH] D16717: [clang-tidy] Add non-constant references in function parameters check.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Thanks! A few comments.


================
Comment at: clang-tidy/google/NonConstReferences.cpp:54
@@ +53,3 @@
+
+  QualType ReferencedType =
+      *Result.Nodes.getNodeAs<QualType>("referenced_type");
----------------
This could be `auto` to avoid duplicating the type name.

================
Comment at: clang-tidy/google/NonConstReferences.cpp:81
@@ +80,3 @@
+        // operator++, operator-- and operation+assignment operators.
+        if (Function->getParamDecl(0) == Parameter) return;
+        break;
----------------
The `return` should be on the next line (`clang-format -style=llvm` should get this right).

================
Comment at: clang-tidy/google/NonConstReferences.cpp:105
@@ +104,3 @@
+  // Some functions use references to comply with established standards.
+  if (Function->getNameAsString() == "swap")
+    return;
----------------
We can avoid calling (the more expensive) `getNameAsString` here:

  if (Function->getDeclName().isIdentifier() && Function->getName() == "swap") ...

================
Comment at: clang-tidy/google/NonConstReferences.cpp:109
@@ +108,3 @@
+  // iostream parameters are typically passed by non-const reference.
+  if (StringRef(ReferencedType.getAsString()).endswith("stream"))
+    return;
----------------
I'd better avoid calling getAsString here as well, but I don't see an easy way to do this.

================
Comment at: test/clang-tidy/google-runtime-references.cpp:1
@@ +1,2 @@
+// RUN: clang-tidy -checks=-*,google-runtime-references %s -- -std=c++11 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
----------------
I think, this test can use the %check_clang_tidy script, since it doesn't seem to do anything the script doesn't support (you'll need to s/CHECK/CHECK-MESSAGES/ to make it work, though).


http://reviews.llvm.org/D16717