[clang-tidy] readability-non-const-parameter: fixit on all function declarations

[AndersRonnholm]

Summary

Fixes issue reported in https://bugs.llvm.org/show_bug.cgi?id=33219

[danielmarjamaki]

LGTM. But others should approve.

[aaron.ballman]

clang-tidy/readability/NonConstParameterCheck.cpp

146

Please do not use auto here, as the type is not spelled out in the initialization.

147

What if the parent is an ObjCMethodDecl instead?

148–152

Don't we need the fixit for *all* declarations of the function? e.g.,

char f(char *c); // Need it here
char f(char *c); // And here

char f(char *c) { // And here
  return *c;
}

[AndersRonnholm]

Fixed comments

[AndersRonnholm]

clang-tidy/readability/NonConstParameterCheck.cpp

147

I don't think this checker handles objective-c

[aaron.ballman]

clang-tidy/readability/NonConstParameterCheck.cpp

143

You should remove the quotes around %0 and drop the getName() -- the diagnostics engine automatically handled NamedDecl subclasses and properly quotes them.

147

I think it does -- it has a matcher for ParmVarDecl, which can be contained by an ObjCMethodDecl.

149

const auto *

151

You can drop the getName() call here as well.

[malcolm.parsons]

Fixed by rL319021?

[AndersRonnholm]

Fixed by rL319021. At least for c/c++ not sure if it handles objective-c.

clang-tidy/readability/NonConstParameterCheck.cpp

for (const auto &It : Parameters) {
     // Unused parameter => there are other warnings about this.
     if (!ParamInfo.IsReferenced)
       continue;
 
     // Parameter can't be const.
     if (!ParamInfo.CanBeConst)
       continue;
 
-    diag(Par->getLocation(), "pointer parameter '%0' can be pointer to const")
+    auto D = diag(Par->getLocation(),
+                  "pointer parameter '%0' can be pointer to const")
              << Par->getName()
              << FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+
+    const DeclContext *Parent = Par->getParentFunctionOrMethod();
+    const auto *FD = dyn_cast<FunctionDecl>(Parent);
+    while (FD) {
+      const auto ParDecl = FD->getParamDecl(Par->getFunctionScopeIndex());
+      if (Par != ParDecl)
+        D << ParDecl->getName()
+          << FixItHint::CreateInsertion(ParDecl->getLocStart(), "const ");
+      FD = FD->getPreviousDecl();
+    }
   }
 }
 
 void NonConstParameterCheck::markCanNotBeConst(const Expr *E,
                                                bool CanNotBeConst) {
   if (!E)
     return;
 

test/clang-tidy/readability-non-const-parameter.cpp

 
 class Derived : public Base {
 public:
   // Ensure there is no false positive for this method. It overrides a method.
   void doStuff(int *p) override {
     int x = *p;
   }
 };
+
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'i' can be
+int declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
+
+
+class D {
+private:
+  int declarationFixit(int *i);
+  // CHECK-FIXES: {{^}}  int declarationFixit(const int *i);{{$}}
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: pointer parameter 'i' can be
+int D::declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}