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

[Richard via cfe-commits]

LegalizeAdulthood added a subscriber: LegalizeAdulthood.

================
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
@@ +14,3 @@
+
+  // warning here; p should be const
+  char f1(char *p) {
----------------
With pointers, there are always two layers of constness:


  - Whether the pointer itself is modified
  - Whether the data pointed to is modified

When you say "p should be const", I read that as the former.

I am pretty sure you intended the latter.  You should be more explicit in your documentation.  The ambiguity would have been resolved if you showed the "after" version of the code that would eliminate the warning.  This is semi-implied by your next example, but it's kinder to the reader to resolve this ambiguity immediately.



================
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:32
@@ +31,3 @@
+  // no warning; Technically, p can be const ("const struct S *p"). But making
+  // p const could be misleading. People might think that it's safe to pass
+  // const data to this function.
----------------
Here you are using "making p const" in the first sense I noted above.

It might help if you say "make `*p` const" when you mean that p points to constant data and use "making `p` const" when you mean that the pointer itself is unmodified.

There is also the wart in C++ that one can declare a function like this:

```
int f(char *p);
```

and define it like this:

```
int f(char *const p) {
  // ...
}
```

My sense is that this is pretty confusing to most programmers who really want the declaration and definition to be textually identical.


http://reviews.llvm.org/D15332