[clang-tidy] Fix readability-non-const-parameter for parameter referenced by an lvalue

The checker missed a check for a case when the parameter is referenced by an lvalue and this could cause build breakages.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D117090

Author: Sockke

clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp

@@ -83,6 +83,20 @@ void NonConstParameterCheck::check(const MatchFinder::MatchResult &Result) {
       for (const auto *Arg : CE->arguments()) {
         markCanNotBeConst(Arg->IgnoreParenCasts(), true);
       }
+      // Data passed by nonconst reference should not be made const.
+      unsigned ArgNr = 0U;
+      if (const auto *CD = CE->getConstructor()) {
+        for (const auto *Par : CD->parameters()) {
+          if (ArgNr >= CE->getNumArgs())
+            break;
+          const Expr *Arg = CE->getArg(ArgNr++);
+          // Is this a non constant reference parameter?
+          const Type *ParType = Par->getType().getTypePtr();
+          if (!ParType->isReferenceType() || Par->getType().isConstQualified())
+            continue;
+          markCanNotBeConst(Arg->IgnoreParenCasts(), false);
+        }
+      }
     } else if (const auto *R = dyn_cast<ReturnStmt>(S)) {
       markCanNotBeConst(R->getRetValue(), true);
     } else if (const auto *U = dyn_cast<UnaryOperator>(S)) {
@@ -93,6 +107,9 @@ void NonConstParameterCheck::check(const MatchFinder::MatchResult &Result) {
     if ((T->isPointerType() && !T->getPointeeType().isConstQualified()) ||
         T->isArrayType())
       markCanNotBeConst(VD->getInit(), true);
+    else if (T->isLValueReferenceType() &&
+             !T->getPointeeType().isConstQualified())
+      markCanNotBeConst(VD->getInit(), false);
   }
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -109,6 +109,10 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Fixed a false positive in :doc:`readability-non-const-parameter
+  <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue
+
+
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/readability-non-const-parameter.rst

@@ -44,3 +44,8 @@ make the function interface safer.
   int f3(struct S *p) {
     *(p->a) = 0;
   }
+
+  // no warning; p is referenced by an lvalue.
+  void f4(int *p) {
+    int &x = *p;
+  }

clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp

@@ -287,3 +287,39 @@ char foo(char *s) {
 }
 char foo(char *s); // 2
 // CHECK-FIXES: {{^}}char foo(const char *s); // 2{{$}}
+
+void lvalueReference(int *p) {
+  // CHECK-MESSAGES-NOT: warning: pointer parameter 'p' can be
+  int &x = *p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:32: warning: pointer parameter 'p' can be
+void constLValueReference(int *p) {
+  // CHECK-FIXES: {{^}}void constLValueReference(const int *p) {{{$}}
+  const int &x = *p;
+}
+
+void lambdaLVRef(int *p) {
+  // CHECK-MESSAGES-NOT: warning: pointer parameter 'p' can be
+  auto foo = [&]() {
+    int &x = *p;
+  };
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:28: warning: pointer parameter 'p' can be
+void lambdaConstLVRef(int *p) {
+  // CHECK-FIXES: {{^}}void lambdaConstLVRef(const int *p) {{{$}}
+  auto foo = [&]() {
+    const int &x = *p;
+  };
+}
+
+struct Temp1 {
+  Temp1(int &i) {
+    i = 10;
+  }
+};
+void constructLVRef(int *p) {
+  // CHECK-MESSAGES-NOT: warning: pointer parameter 'p' can be
+  Temp1 t(*p);
+}