Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 42164. danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. Moved warning to clang-tidy. In this patch I am more careful about function calls. Sometimes when it is technically possible to use const it's still not a good

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a subscriber: alexfh. danielmarjamaki added a reviewer: alexfh. danielmarjamaki added a comment. Alexander: I add you as reviewer since this was moved to clang-tidy. http://reviews.llvm.org/D12359 ___ cfe-commits mailing list

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. In http://reviews.llvm.org/D12359#304772, @danielmarjamaki wrote: > Moved warning to clang-tidy. > > In this patch I am more careful about function calls. Sometimes when it is > technically possible to use const it's still not a good idea. > > For instance when

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

alexfh added a comment. Wow, that's a patch with history ;) My first comment is that the `misc` module mostly consists of checks that are safe to turn on by default. This check does not necessarily meet this bar, so I'd better move it to `readability`. You could also send a new patch, since

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki abandoned this revision. danielmarjamaki added a comment. I have created a new revision: http://reviews.llvm.org/D15332 http://reviews.llvm.org/D12359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. Does anybody else think that this should be moved to clang-tidy? I believe that the noise level is very low when I scan various open source projects. My script tries to run clang on all projects in the debian

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

On Wed, Nov 18, 2015 at 7:41 AM, Daniel Marjamäki < cfe-commits@lists.llvm.org> wrote: > danielmarjamaki marked an inline comment as done. > danielmarjamaki added a comment. > > Does anybody else think that this should be moved to clang-tidy? > Yep > > I believe that the noise level is very

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 40040. danielmarjamaki added a comment. Moved NonConstUse from VarDecl to ParmVarDecl to avoid waste of memory http://reviews.llvm.org/D12359 Files: include/clang/AST/Decl.h include/clang/Basic/DiagnosticGroups.td

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. In http://reviews.llvm.org/D12359#287522, @rsmith wrote: > Why does this construct justify the compiler emitting a warning? It seems to > be reporting a fact about the code rather than a bug, and as there are many > coding styles where variables are not

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

rsmith added a comment. Why does this construct justify the compiler emitting a warning? It seems to be reporting a fact about the code rather than a bug, and as there are many coding styles where variables are not routinely marked as const whenever possible, this appears to be checking that

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 39679. danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. fixes of review comments http://reviews.llvm.org/D12359 Files: include/clang/AST/Decl.h include/clang/Basic/DiagnosticGroups.td

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

aaron.ballman added inline comments. Comment at: lib/Parse/ParseStmt.cpp:376 @@ -375,3 +375,3 @@ /// \brief Parse an expression statement. StmtResult Parser::ParseExprStatement() { // If a case keyword is missing, this is where it should be inserted. > I

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 37969. danielmarjamaki added a comment. Fixed a FP. Triaged warnings (random selection of warnings): ftp://ftp.se.debian.org/debian/pool/main/a/aespipe/aespipe_2.4c.orig.tar.bz2 ./aespipe.c:467:43: warning: parameter 'keyStr' can be const

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 37728. danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. Fix FN for code: const char *ret(char *p) { return p ? p : ""; } http://reviews.llvm.org/D12359 Files: include/clang/AST/Decl.h

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. In http://reviews.llvm.org/D12359#233152, @sberg wrote: > causes false positive for > > char * f(char *); > char * g(char * p) { return f(p); } > Sorry for replying this late. This should work in latest patch. Comment at:

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 37582. danielmarjamaki marked 5 inline comments as done. danielmarjamaki added a comment. One more work in progress patch. I have moved the NonConstUse to VarDecl. I turned on Wnonconst-parameter by default. This had a very positive effect; I saw

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 37584. danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. Minor fixes from previous patch. Added some more testcases. http://reviews.llvm.org/D12359 Files: include/clang/AST/Decl.h

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 37223. danielmarjamaki added a comment. Minor cleanups and refactorings http://reviews.llvm.org/D12359 Files: include/clang/AST/Decl.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:201 @@ -200,1 +200,3 @@ +def warn_nonconst_parameter : Warning<"parameter %0 can be const">, + InGroup, DefaultIgnore; def warn_unused_variable : Warning<"unused variable %0">,

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. Thanks! Very good comments. I will look at the comments asap. but unfortunately I don't have time right now. I expect that I can continue working on this warning in a few weeks. Comment at: include/clang/AST/DeclBase.h:279 @@ +278,3 @@ + ///

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Thank you for continuing to work on this, I think it's a good diagnostic to have. There are still quite a few unanswered questions in the phab thread. Also, the patch appears to be missing tests, which might help to clarify

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 35482. danielmarjamaki marked 9 inline comments as done. danielmarjamaki added a comment. With the previous patch there was much noise when building Clang. I have fixed many false positives with improved handling of C++ code. However I have not been

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. > > > The checker isn't currently path sensitive as it doesn't pay attention > > > > > > > to control flow graphs or how pointer values flow through a function > > > > > > > body. I suppose this is a matter of scope more than anything; I see a >

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. The full output log I got when building clang was very big. 47MB. Roughly half of that is lines saying "In file included from". I also saw false positives for constructors. The initialisation list is not considered properly. I reduced the output with this grep

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. After looking at 5 of the warnings for Clang.. it is obvious that there are lots of FP for this code. I will fix these so a better log for Clang can be shown. http://reviews.llvm.org/D12359 ___ cfe-commits mailing

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

On Wed, Sep 2, 2015 at 2:19 AM, Daniel Marjamäki wrote: > danielmarjamaki added a comment. > >> > > The checker isn't currently path sensitive as it doesn't pay attention > >> > >> > > >> > >> > > to control flow graphs or how pointer values flow through a function

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. In http://reviews.llvm.org/D12359#236334, @aaron.ballman wrote: > I have concerns about this being a frontend warning. The true positive rate > seems rather high given the benign nature of the diagnostic, and the false > negative rate is quite high. This seems

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

On Mon, Aug 31, 2015 at 2:58 PM, Daniel Marjamäki wrote: > danielmarjamaki added a comment. > > In http://reviews.llvm.org/D12359#236334, @aaron.ballman wrote: > >> I have concerns about this being a frontend warning. The true positive rate >> seems rather high

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 33575. danielmarjamaki added a comment. Fixed 2 false positives found in python source code. http://reviews.llvm.org/D12359 Files: include/clang/AST/DeclBase.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki added a comment. I saw a FP when I looked through the logs today... Example code: void dostuff(int *); void f(int *p) { #ifdef X dostuff(p); #endif if (*p == 0) {} } As far as I know there is nothing I can do about this in the checker. A user could

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki updated this revision to Diff 33307. danielmarjamaki added a comment. Thanks! I fixed that FP. http://reviews.llvm.org/D12359 Files: include/clang/AST/DeclBase.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

sberg added a subscriber: sberg. sberg added a comment. causes false positive for char * f(char *); char * g(char * p) { return f(p); } http://reviews.llvm.org/D12359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

danielmarjamaki created this revision. danielmarjamaki added a subscriber: cfe-commits. This is a new warning for Clang. It will warn when a pointer parameter can be const. The design is inspired by the Wunused-parameter warning. I have tested this on many debian packages. In 2151 projects