[PATCH] D83864: [ClangTidy] Fix false positives of readability-non-const-parameters check

[Jacques Lucke via Phabricator via cfe-commits]

JacquesLucke created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We recently started using Clang Tidy on the Blender source. We found a 
couple of false positives of the `readability-non-const-parameters` check. 
One of those is fixed by this patch.

  struct MyStruct {
float *values;
  };
  
  void myfunc(float *values) {
MyStruct mystruct = {values};
  }

Without this patch, Clang Tidy reports that the `values` parameter of
`myfunc` can be const. It does not recognize that it is assigned to
a non-const data member of `MyStruct`.

Note, this is the first time I'm working with the LLVM source code.
So please let me know if I'm doing something wrong.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83864

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


Index: clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
@@ -99,7 +99,6 @@
 for (const auto *F : D->fields()) {
   if (InitNr >= ILE->getNumInits())
 break;
-
   const auto *Init = ILE->getInit(InitNr++);
   if (!F->getType().isConstQualified())
 markCanNotBeConst(Init, true);


Index: clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
@@ -99,7 +99,6 @@
 for (const auto *F : D->fields()) {
   if (InitNr >= ILE->getNumInits())
 break;
-
   const auto *Init = ILE->getInit(InitNr++);
   if (!F->getType().isConstQualified())
 markCanNotBeConst(Init, true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83864: [ClangTidy] Fix false positives of readability-non-const-parameters check

[Jacques Lucke via Phabricator via cfe-commits]

JacquesLucke updated this revision to Diff 278154.
JacquesLucke added a comment.

For some reason `arc` did not upload all the changes. Maybe because I cloned 
from github, will check. Anyway, here is the patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83864/new/

https://reviews.llvm.org/D83864

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
@@ -217,11 +217,20 @@
 // Don't warn about nonconst record pointers that can be const.
 struct XY {
   int *x;
-  int *y;
+  const int *y;
 };
 void recordpointer(struct XY *xy) {
   *(xy->x) = 0;
 }
+// CHECK-MESSAGES: :[[@LINE+1]]:21: warning: pointer parameter 'y' can be
+void initlist1(int *y) {
+  // CHECK-FIXES: {{^}}void initlist1(const int *y) {{{$}}
+  XY xy = {nullptr, y};
+}
+// Don't warn when pointer is assigned to non-const struct member.
+void initlist2(int *x) {
+  XY xy = {x};
+}
 
 class C {
 public:
Index: clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
@@ -92,6 +92,19 @@
 }
   } else if (const auto *VD = Result.Nodes.getNodeAs("Mark")) {
 const QualType T = VD->getType();
+if (T->isRecordType()) {
+  if (const auto *ILE = dyn_cast_or_null(VD->getInit())) {
+const auto *D = T->getAs()->getDecl();
+unsigned InitNr = 0U;
+for (const auto *F : D->fields()) {
+  if (InitNr >= ILE->getNumInits())
+break;
+  const auto *Init = ILE->getInit(InitNr++);
+  if (!F->getType().isConstQualified())
+markCanNotBeConst(Init, true);
+}
+  }
+}
 if ((T->isPointerType() && !T->getPointeeType().isConstQualified()) ||
 T->isArrayType())
   markCanNotBeConst(VD->getInit(), true);


Index: clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp
@@ -217,11 +217,20 @@
 // Don't warn about nonconst record pointers that can be const.
 struct XY {
   int *x;
-  int *y;
+  const int *y;
 };
 void recordpointer(struct XY *xy) {
   *(xy->x) = 0;
 }
+// CHECK-MESSAGES: :[[@LINE+1]]:21: warning: pointer parameter 'y' can be
+void initlist1(int *y) {
+  // CHECK-FIXES: {{^}}void initlist1(const int *y) {{{$}}
+  XY xy = {nullptr, y};
+}
+// Don't warn when pointer is assigned to non-const struct member.
+void initlist2(int *x) {
+  XY xy = {x};
+}
 
 class C {
 public:
Index: clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
@@ -92,6 +92,19 @@
 }
   } else if (const auto *VD = Result.Nodes.getNodeAs("Mark")) {
 const QualType T = VD->getType();
+if (T->isRecordType()) {
+  if (const auto *ILE = dyn_cast_or_null(VD->getInit())) {
+const auto *D = T->getAs()->getDecl();
+unsigned InitNr = 0U;
+for (const auto *F : D->fields()) {
+  if (InitNr >= ILE->getNumInits())
+break;
+  const auto *Init = ILE->getInit(InitNr++);
+  if (!F->getType().isConstQualified())
+markCanNotBeConst(Init, true);
+}
+  }
+}
 if ((T->isPointerType() && !T->getPointeeType().isConstQualified()) ||
 T->isArrayType())
   markCanNotBeConst(VD->getInit(), true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits