[PATCH] D15332: new clang-tidy checker readability-non-const-parameter

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 14 07:11:48 PDT 2016


hokein added inline comments.

================
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:24
@@ +23,3 @@
+
+  // C++ constructor..
+  Finder->addMatcher(cxxConstructorDecl().bind("Ctor"), this);
----------------
Extra `.` at the end.

================
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:124
@@ +123,3 @@
+void NonConstParameterCheck::diagnoseNonConstParameters() {
+  for (auto It : Parameters) {
+    const ParmVarDecl *Par = It.first;
----------------
const auto &It

================
Comment at: clang-tidy/readability/NonConstParameterCheck.h:53
@@ +52,3 @@
+  /// and CanNotBeConst is true the Parameter is marked as not-const.
+  /// The CanNotBeConst are updated as sub expressions are visited.
+  void markCanNotBeConst(const Expr *E, bool CanNotBeConst);
----------------
s/are/is

================
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:6
@@ +5,3 @@
+
+Finds function parameters that should be const. When const is used properly,
+many mistakes can be avoided. Advantages when using const properly:
----------------
Looks like what the document says isn't consistent with the check, since the check only finds non-const pointer parameter. 

================
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3
@@ +2,3 @@
+
+// Currently the checker only warns about pointer arguments.
+//
----------------
It makes sense to move this document to the `rst`, I think.

================
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:15
@@ +14,3 @@
+
+// CHECK-MESSAGES: :[[@LINE+1]]:29: warning: parameter 'last' can be const [readability-non-const-parameter]
+void warn1(int *first, int *last) {
----------------
Just keep the whole warning message in the first CHECK-MESSAGE.
And remove `[readability-non-const-parameter]` in others CHECK-MESSAGE for keeping the line short. 

================
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:219
@@ +218,3 @@
+public:
+  C(int *p) : p(p) {}
+private:
----------------
Please add a test case:

```
class C {
public:
  C(int *p) : p(p) {}
private:
  const int *p;
};
```

BTW, does the check support class method?


http://reviews.llvm.org/D15332





More information about the cfe-commits mailing list