[PATCH] D19534: [clang-tidy] new google-default-arguments check

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon May 2 01:07:05 PDT 2016


hokein added inline comments.

================
Comment at: clang-tidy/google/DefaultArgumentsCheck.cpp:31
@@ +30,3 @@
+  diag(MatchedDecl->getLocation(),
+       "default argument given for virtual or override method.");
+}
----------------
Usually, clang-tidy's warning message is not a sentence, so remove the `.` at the end. 

How about `default arguments on virtual or override methods are prohibited`?

================
Comment at: clang-tidy/google/GoogleTidyModule.cpp:40
@@ -38,1 +39,3 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<DefaultArgumentsCheck>(
+        "google-default-arguments");
----------------
Please keep the alphabetical order.

================
Comment at: docs/ReleaseNotes.rst:92
@@ +91,3 @@
+
+  Flags default arguments in vitual methods.
+
----------------
s/vitual/virtual

================
Comment at: docs/clang-tidy/checks/google-default-arguments.rst:6
@@ +5,3 @@
+
+Checks that default parameters are not given for virtual methods.
+
----------------
I'm a little confused about the words here. Indeed, the `google-default-arguments` checks the default parameter given for virtual methods.


================
Comment at: test/clang-tidy/google-default-arguments.cpp:5
@@ +4,3 @@
+  virtual void f(int I, int J = 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: default argument given for virtual or override method
+};
----------------
The the first warning message you need check the all message including the check name.


http://reviews.llvm.org/D19534





More information about the cfe-commits mailing list