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

2016-08-23 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-18 Thread Alexander Kornienko via cfe-commits
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

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

2016-08-18 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-18 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-18 Thread 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(),

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

2016-08-16 Thread Alexander Kornienko via cfe-commits
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())

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

2016-08-15 Thread Daniel Marjamäki via cfe-commits
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); +

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

2016-08-12 Thread Piotr Padlewski via cfe-commits
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

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

2016-08-10 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-10 Thread Daniel Marjamäki via cfe-commits
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-

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

2016-08-10 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-10 Thread Daniel Marjamäki via cfe-commits
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

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

2016-08-10 Thread Daniel Marjamäki via cfe-commits
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

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

2016-04-01 Thread Eugene Zelenko via cfe-commits
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. ---

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

2016-04-01 Thread Alexander Kornienko via cfe-commits
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

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

2016-03-22 Thread Richard via cfe-commits
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) {{{$}}

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

2016-03-22 Thread Daniel Marjamäki via cfe-commits
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) {{{$}} +

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

2016-03-19 Thread Richard Smith via cfe-commits
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

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

2016-03-15 Thread Daniel Marjamäki via cfe-commits
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

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

2016-03-15 Thread Daniel Marjamäki via cfe-commits
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

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

2016-03-14 Thread Haojian Wu via cfe-commits
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

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

2016-03-14 Thread Daniel Marjamäki via cfe-commits
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

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

2016-03-08 Thread Daniel Marjamäki via 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, ---

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

2016-03-08 Thread Daniel Marjamäki via cfe-commits
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

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

2016-03-08 Thread Daniel Marjamäki via cfe-commits
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

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

2016-03-08 Thread Daniel Marjamäki via cfe-commits
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: >

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

2016-02-17 Thread Richard via cfe-commits
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

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

2016-02-17 Thread Aaron Ballman via cfe-commits
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) { >

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

2016-02-16 Thread Daniel Marjamäki via cfe-commits
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

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

2016-02-15 Thread Richard via cfe-commits
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

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

2016-02-15 Thread Richard via cfe-commits
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:

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

2016-02-15 Thread Daniel Marjamäki via cfe-commits
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

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

2016-02-15 Thread Daniel Marjamäki via cfe-commits
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

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

2015-12-10 Thread Eugene Zelenko via cfe-commits
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

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

2015-12-09 Thread Daniel Marjamäki via cfe-commits
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

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

2015-12-09 Thread Eugene Zelenko via cfe-commits
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

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

2015-12-09 Thread Daniel Marjamäki via cfe-commits
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

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

2015-12-09 Thread Alexander Kornienko via cfe-commits
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 ___

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

2015-12-09 Thread Daniel Marjamäki via cfe-commits
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

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

2015-12-09 Thread Daniel Marjamäki via cfe-commits
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

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

2015-12-08 Thread Alexander Kornienko via cfe-commits
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

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

2015-12-08 Thread Aaron Ballman via cfe-commits
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