This revision was automatically updated to reflect the committed changes.
Closed by commit rL279507: [clang-tidy] readability-non-const-parameter: add
new check that warns when… (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D15332?vs=68531&id=68963#toc
Reposi
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:104
@@ +103,3 @@
+ const QualType T = Parm->getType();
+ if (!T->isPointerType() || T->getPointeeType().isCo
danielmarjamaki updated this revision to Diff 68531.
danielmarjamaki added a comment.
Fixed review comments about formatting in doc
https://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clang-tidy/readability/NonCons
danielmarjamaki updated this revision to Diff 68528.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
Fixed review comments
https://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clan
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(),
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:95-98
@@ +94,6 @@
+const QualType T = VD->getType();
+if (T->isPointerType() && !T->getPointeeType().isConstQualified())
danielmarjamaki added inline comments.
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);
+
Prazek added a subscriber: Prazek.
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);
+els
danielmarjamaki updated this revision to Diff 67506.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
added this check to the release notes. run clang-format.
https://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readabilit
danielmarjamaki marked 2 inline comments as done.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:245
@@ +244,3 @@
+ C c(p);
+}
+
I have added tests and fixed FPs in latest diff.
Comment at: test/clang-tidy/readability-non-const-
danielmarjamaki updated this revision to Diff 67504.
danielmarjamaki added a comment.
fixed more review comments
https://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clang-tidy/readability/NonConstParameterCheck.h
danielmarjamaki marked 10 inline comments as done.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:117-135
@@ +116,21 @@
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: pointer parameter 'p' can be
+int return1(int *p) {
+ // CHECK-FIXES: {{^}}int return1(const int
danielmarjamaki updated this revision to Diff 67500.
danielmarjamaki added a comment.
Fix patch so it can be applied in latest trunk. Tried to fix review comments.
https://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
Eugene.Zelenko added a comment.
Please also mention check in docs/ReleaseNotes.rst.
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
@@ +14,3 @@
+
+ // warning here; the declaration "const char *p" would make the function
+ // interface safer.
---
alexfh added inline comments.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:44
@@ +43,3 @@
+addParm(Parm);
+ } else if (const CXXConstructorDecl *Ctor =
+ Result.Nodes.getNodeAs("Ctor")) {
`const auto *Ctor`
C
LegalizeAdulthood added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:116-134
@@ +115,21 @@
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: parameter 'p' can be const
+int return1(int *p) {
+ // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
danielmarjamaki added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:116-134
@@ +115,21 @@
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: parameter 'p' can be const
+int return1(int *p) {
+ // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+
rsmith added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:116-134
@@ +115,21 @@
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: parameter 'p' can be const
+int return1(int *p) {
+ // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+ return
danielmarjamaki updated this revision to Diff 50705.
danielmarjamaki marked 3 inline comments as done.
danielmarjamaki added a comment.
Fixed review comments
http://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clang
danielmarjamaki marked 12 inline comments as done.
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:6
@@ +5,3 @@
+
+Finds function parameters that should be const. When const is used properly,
+many mistakes can be avoided. Advantages when using const properl
hokein added inline comments.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:24
@@ +23,3 @@
+
+ // C++ constructor..
+ Finder->addMatcher(cxxConstructorDecl().bind("Ctor"), this);
Extra `.` at the end.
Comment at: clang-tidy/read
danielmarjamaki added a comment.
ping.
http://reviews.llvm.org/D15332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki marked 2 inline comments as done.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:4
@@ +3,3 @@
+// Currently the checker only warns about pointer arguments.
+//
+// It can be defined both that the data is const and that the pointer is const,
---
danielmarjamaki marked 7 inline comments as done.
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:16
@@ +15,3 @@
+ // warning here; the declaration "const char *p" would make the function
+ // interface safer.
+ char f1(char *p)
thanks. I
danielmarjamaki updated this revision to Diff 50032.
danielmarjamaki added a comment.
Updated documentation
http://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clang-tidy/readability/NonConstParameterCheck.h
clang
danielmarjamaki added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3
@@ +2,3 @@
+
+// Currently the checker only warns about pointer arguments.
+//
LegalizeAdulthood wrote:
> danielmarjamaki wrote:
> > LegalizeAdulthood wrote:
>
LegalizeAdulthood added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3
@@ +2,3 @@
+
+// Currently the checker only warns about pointer arguments.
+//
danielmarjamaki wrote:
> LegalizeAdulthood wrote:
> > How hard is it to extend
On Wed, Feb 17, 2016 at 2:39 AM, Daniel Marjamäki
wrote:
> danielmarjamaki marked 2 inline comments as done.
>
>
> Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
> @@ +14,3 @@
> +
> + // warning here; p should be const
> + char f1(char *p) {
>
danielmarjamaki marked 2 inline comments as done.
Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
@@ +14,3 @@
+
+ // warning here; p should be const
+ char f1(char *p) {
LegalizeAdulthood wrote:
> With pointers, there are always two lay
LegalizeAdulthood added inline comments.
Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3
@@ +2,3 @@
+
+// Currently the checker only warns about pointer arguments.
+//
How hard is it to extend it to references?
Certainly the confusion about what
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:
danielmarjamaki added a comment.
In http://reviews.llvm.org/D15332#352556, @danielmarjamaki wrote:
>
> I see no warning when running on Clang source code (files in
> llvm/tools/clang/lib/...).
For information I rerun with --header-filter=* and got some results.. I will
triage those..
ht
danielmarjamaki updated this revision to Diff 47962.
danielmarjamaki added a comment.
Run this check on C++ code also. I have rerun the add_new_check python script.
Minor code cleanups.
I have run this on the debian packages again. In 1806 projects there were 9691
warnings. I have so far triage
Eugene.Zelenko added a comment.
In http://reviews.llvm.org/D15332#306831, @danielmarjamaki wrote:
> In http://reviews.llvm.org/D15332#306097, @Eugene.Zelenko wrote:
>
> > This check partially implements PR19419. Could it be extended to variables?
>
>
> Yes, I don't see any technical reasons why t
danielmarjamaki added a comment.
In http://reviews.llvm.org/D15332#306097, @Eugene.Zelenko wrote:
> This check partially implements PR19419. Could it be extended to variables?
Yes, I don't see any technical reasons why that would not work. However
politically, some people like const variables
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.
This check partially implements PR19419. Could it be extended to variables?
http://reviews.llvm.org/D15332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
danielmarjamaki marked 3 inline comments as done.
danielmarjamaki added a comment.
In http://reviews.llvm.org/D15332#305976, @alexfh wrote:
> Please add tests with templates and macros. Please also run this on
> LLVM/Clang to ensure a) it doesn't crash; b) the number of warnings is sane;
> c) a
alexfh added a comment.
Please add tests with templates and macros. Please also run this on LLVM/Clang
to ensure a) it doesn't crash; b) the number of warnings is sane; c) a
representative sample of warnings doesn't contain false positives.
http://reviews.llvm.org/D15332
___
danielmarjamaki marked 12 inline comments as done.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:44
@@ +43,3 @@
+ if (B->isAssignmentOp())
+markCanNotBeConst(B, false);
+} else if (const auto *CE = dyn_cast(S)) {
I tried to unify l
danielmarjamaki updated this revision to Diff 42292.
danielmarjamaki added a comment.
I have tried to fix the comments.
This patch is mostly untested. I have only run 'make check-all'. I am running
more tests right now.
http://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists
alexfh added inline comments.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:20
@@ +19,3 @@
+void NonConstParameterCheck::registerMatchers(MatchFinder *Finder) {
+ // TODO: This checker doesn't handle C++ to start with. There are problems
+ // for example with par
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:43
@@ +42,3 @@
+ Finder->addMatcher(returnStmt().bind("return"), this);
+}
+
I think it may be an improvement to unify all of the matchers that will
eventually be
42 matches
Mail list logo