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

[Daniel Marjamäki via cfe-commits]

danielmarjamaki marked 6 inline comments as done.

================
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:95-98
@@ +94,6 @@
+    const QualType T = VD->getType();
+    if (T->isPointerType() && !T->getPointeeType().isConstQualified())
+      markCanNotBeConst(VD->getInit(), true);
+    else if (T->isArrayType())
+      markCanNotBeConst(VD->getInit(), true);
+  }
----------------
alexfh wrote:
> danielmarjamaki wrote:
> > Prazek wrote:
> > > This looks like it could be in the same if.
> > Yes it could. But would it make the code more or less readable? It wouldn't be a 1-line condition anymore then.
> I also think that it makes sense to merge the conditions. The problem with the current code is that it is suspicious ("Why is the same action is done in two branches? Is it a bug?"). One line condition vs two lines seems secondary in this case.
ok

================
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:103
@@ +102,3 @@
+void NonConstParameterCheck::addParm(const ParmVarDecl *Parm) {
+  // Only add nonconst integer/float pointer parameters.
+  const QualType T = Parm->getType();
----------------
alexfh wrote:
> This seems too strict. What about other primitive types? 
I am not sure which type you are talking about. As far as I see we're writing warnings about bool,char,short,int,long,long long,float,double,long double,enum pointers.

I have intentionally avoided records now to start with. It should be added, but we need to be more careful when we do it.


================
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:210
@@ +209,3 @@
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'p' can be
+int functionpointer2(int *p)
+{
----------------
alexfh wrote:
> Put braces on the previous line, please. A few other instances below.
sorry .. of course I should run clang-format on this.


https://reviews.llvm.org/D15332