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

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..

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-14 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. ping http://reviews.llvm.org/D13126 ___ 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-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-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

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

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-22 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In http://reviews.llvm.org/D13126#379306, @zaks.anna wrote: > Why is there such a large jump in the number of warnings reported in the last > patch iteration? > It went from "1678 projects where scanned. In total I got 124 warnings" to > "In 2215 projects it

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-22 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext , const Expr *E, + unsigned long long Val)

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. Here are some false positives I have marked... let me know if you want more... I have marked this as a FP: URL: ftp://ftp.se.debian.org/debian/pool/main/m/m17n-lib/m17n-lib_1.7.0.orig.tar.gz File: m17n-lib-1.7.0/src/mtext.c Line 1946 Code: int

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext , const Expr *E, + unsigned long long Val)

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 51402. danielmarjamaki added a comment. Minor fixes of review comments. - Improved comments about the checker in the source code. - Improved capitalization and punctuation in error messages. - Renamed "canBe..." functions and changed their return

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-21 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. ping http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 3 inline comments as done. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:12 @@ +11,3 @@ +// +// ConversionChecker generates a subset of the warnings that are reported by +// Wconversion. It is designed to be an alternative to Wconversion.

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-03-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext , const Expr *E, + unsigned long long Val)

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

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] D13126: New static analyzer checker for loss of sign/precision

2016-03-30 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 52029. danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. Minor fixes of review comments. Renamed functions to "isNegative" and "isGreaterEqual" and return bool. Updated comment. Added testcases for false positives I

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67470. danielmarjamaki added a comment. Make sure patch can be applied in latest trunk. Ping. https://reviews.llvm.org/D13126 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt

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] D13126: New static analyzer checker for loss of sign/precision

2016-08-12 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 3 inline comments as done. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:67 @@ +66,3 @@ + const Stmt *Parent = PM.getParent(Cast); + if (!Parent) +return; I get assertion failure then when running this test:

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-08-12 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67817. danielmarjamaki added a comment. fixed review comments https://reviews.llvm.org/D13126 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt

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-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 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 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:

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D21134#508524, @jroelofs wrote: > I think the replacement is wrong for something like: > > int *arr; int offs1, offs2; > offs1[arr + offs2] = 42; > > > which I think would give: > > int *arr; int offs1, offs2; > arr +

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. In https://reviews.llvm.org/D21134#508511, @aaron.ballman wrote: > Is there a reason we don't want this check to be a part of the clang > frontend, rather than as a clang-tidy check? I discussed this with

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67322. danielmarjamaki added a comment. take care of review comments https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MisplacedArrayIndexCheck.cpp

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 2 inline comments as done. Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:31 @@ +30,3 @@ + x[10] = 0; + dostuff(0[0 + ABC]); +} aaron.ballman wrote: > Why is this considered to be "normal syntax" and not worth getting a

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 4 inline comments as done. danielmarjamaki added a comment. as far as I see the only unsolved review comment now is about macros. if it can be handled better. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:28-29 @@ +27,4 @@ +

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67338. danielmarjamaki added a comment. ran clang-format https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MisplacedArrayIndexCheck.cpp clang-tidy/readability/MisplacedArrayIndexCheck.h

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-09 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67337. danielmarjamaki added a comment. More generic diagnostic. Diagnose all integerType[pointerType] expressions. https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MisplacedArrayIndexCheck.cpp

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-08 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43 @@ +42,3 @@ + +static StringRef getAsString(const MatchFinder::MatchResult , + const Expr *E) { alexfh wrote: > Looks like this

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-08 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki set the repository for this revision to rL LLVM. danielmarjamaki updated this revision to Diff 67144. danielmarjamaki added a comment. Fixed review comments. Repository: rL LLVM https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-08 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 67145. danielmarjamaki added a comment. Cleanup my previous commit. Ran clang-format. https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt

Re: [PATCH] D24238: StaticAnalyzer CastToStruct : No memory corruption when casting array to struct

2016-09-12 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. hmm.. I don't actually care much about this specific check at the moment. I saw other false positives (unreachable code) and thought that this check made the analyzer think there was corrupted memory. Now I can't

Re: [PATCH] D24238: StaticAnalyzer CastToStruct : No memory corruption when casting array to struct

2016-09-12 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. If the -fno-strict-aliasing would fix this warning then it would be OK. If you are telling me that this CastToStruct check is about alignment or endianness then I think the message is highly misleading. We should rewrite the message. In general, using char

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-09-12 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL281206: [clang-tidy] readability-misplaced-array-index: add new check that warns when… (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D21134?vs=69558=70994#toc

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-16 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added a reviewer: alexfh. danielmarjamaki added a subscriber: cfe-commits. Herald added subscribers: mgorny, beanz. This is a new check that warns about redundant variable declarations. https://reviews.llvm.org/D24656 Files:

[PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-09-26 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added subscribers: cfe-commits, NoQ, zaks.anna, a.sidorin, xazax.hun. danielmarjamaki set the repository for this revision to rL LLVM. This patch fixes false positives for vardecls that are technically unreachable but they are needed. ```

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-10-05 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. Ping. I am not very happy about how I detect multivariable declarations. But I really don't see any such info in the VarDecl. For instance, the AST dump does not show this info. https://reviews.llvm.org/D24656

[PATCH] D19586: Misleading Indentation check

2016-10-05 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. > MisleadingIndentationCheck.cpp:20 > + > +void MisleadingIndentationCheck::danglingElseCheck( > +const MatchFinder::MatchResult ) { There is no handling of tabs and spaces by danglingElseCheck as far as I see. The "if" might for example be indented

[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-10-05 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki set the repository for this revision to rL LLVM. danielmarjamaki updated this revision to Diff 73633. danielmarjamaki added a comment. Add new flag : -Wshift-op-parentheses-mul This is not enabled by default. Repository: rL LLVM https://reviews.llvm.org/D24861 Files:

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 73926. danielmarjamaki added a comment. Refactoring. https://reviews.llvm.org/D25326 Files: include/clang/Analysis/ProgramPoint.h

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL283554: [analyzer] Don't merge different return nodes in ExplodedGraph (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D25326?vs=73926=73947#toc Repository: rL LLVM

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D25326#564283, @NoQ wrote: > In https://reviews.llvm.org/D25326#564239, @danielmarjamaki wrote: > > > ok. As far as I see it's not trivial to know which ReturnStmt there was > > when CallExitBegin is created. > > > We're in

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D25326#564291, @danielmarjamaki wrote: > In https://reviews.llvm.org/D25326#564283, @NoQ wrote: > > > In https://reviews.llvm.org/D25326#564239, @danielmarjamaki wrote: > > > > > ok. As far as I see it's not trivial to know which

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added reviewers: NoQ, zaks.anna, dcoughlin, a.sidorin, xazax.hun. danielmarjamaki added a comment. adding reviewers. Repository: rL LLVM https://reviews.llvm.org/D25326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D25326#564082, @NoQ wrote: > Funny facts about the Environment: > > (a) Environment allows taking SVals of ReturnStmt, which is not an > expression, by transparently converting it into its sub-expression. In fact, > it only stores

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-06 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added subscribers: cfe-commits, dcoughlin, NoQ. danielmarjamaki set the repository for this revision to rL LLVM. Returns when calling an inline function should not be merged in the ExplodedGraph unless they are same. Background post on

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-07 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. ok. As far as I see it's not trivial to know which ReturnStmt there was when CallExitBegin is created. Do you suggest that I move it or that I try to lookup the ReturnStmt? I guess it can be looked up by looking in the predecessors in the ExplodedGraph? >

Re: [PATCH] D24232: Wbitfiled-constant-conversion : allow ~0 , ~(1<<A), etc

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a subscriber: cfe-commits. danielmarjamaki added a comment. ping https://reviews.llvm.org/D24232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. > Will be good idea to detect redundant function prototypes. Yes. Should that have the same ID though? Is it better to have one readability-redundant-declaration or two separate readability-redundant-variable-declaration and

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 71775. danielmarjamaki added a comment. run clang-format on test. add release notes. https://reviews.llvm.org/D24656 Files: clang-tidy/readability/CMakeLists.txt

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. https://reviews.llvm.org/D24656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. > However, I think this check should be part of Clang diagnostics. GCC has > -Wredundant-decls for a long time. I am not against that. What is the criteria? When should it be in the compiler and when should it be in clang-tidy? Personally I think it's

Re: [PATCH] D16309: Use getCharWidth() instead of magic number

2016-09-19 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. ping. https://reviews.llvm.org/D16309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

2016-09-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 72252. danielmarjamaki added a comment. Updated CFGBuilder::VisitDoStmt https://reviews.llvm.org/D24759 Files: lib/Analysis/CFG.cpp test/Analysis/uninit-sometimes.cpp test/Analysis/unreachable-code-path.c Index:

Re: [PATCH] D16309: Use getCharWidth() instead of magic number

2016-09-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Fixed by r282242 https://reviews.llvm.org/D16309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

2016-09-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. My change is causing a false negative in the test/Analysis/uninit-sometimes.cpp. As far as I see my change anyway makes the unoptimized CFG better. https://reviews.llvm.org/D24759 ___ cfe-commits mailing list

[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-09-23 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added reviewers: dblaikie, rtrieu. danielmarjamaki added a subscriber: cfe-commits. danielmarjamaki set the repository for this revision to rL LLVM. This patch makes Clang warn about following code: a = (b * c >> 2); It might be a good

Re: [PATCH] D24507: Add attribute for return values that shouldn't be cast to bool

2016-09-20 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a subscriber: danielmarjamaki. Comment at: include/clang/Basic/AttrDocs.td:2055 @@ -2054,1 +2054,3 @@ } +def WarnImpcastToBoolDocs : Documentation { + let Category = DocCatFunction; I saw your email on cfe-dev. This sounds like a good idea

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-16 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. https://reviews.llvm.org/D24656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-16 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 71614. danielmarjamaki added a comment. minor fixes https://reviews.llvm.org/D24656 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantDeclarationCheck.cpp

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-16 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. For information, I am testing this on debian packages right now. I will see the results next week. Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:22 @@ +21,3 @@ +void RedundantDeclarationCheck::registerMatchers(MatchFinder

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-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

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

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=68963#toc

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-24 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 6 inline comments as done. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:57 @@ +56,2 @@ +} // namespace tidy +} // namespace clang I removed hasMacroId() and use fixit::getText(). The replacements look good now.

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-24 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 69113. danielmarjamaki added a comment. fixed review comments https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MisplacedArrayIndexCheck.cpp clang-tidy/readability/MisplacedArrayIndexCheck.h

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-29 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 69558. danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. fix review comments https://reviews.llvm.org/D21134 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MisplacedArrayIndexCheck.cpp

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-08-29 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 4 inline comments as done. danielmarjamaki added a comment. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-09-28 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL282574: [StaticAnalyzer] Fix false positives for vardecls that are technically… (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D24905?vs=72775=72793#toc Repository:

Re: [PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 72797. danielmarjamaki added a comment. Don't write warning for multiplication in LHS of <<. Often the execution order is not important. https://reviews.llvm.org/D24861 Files:

Re: [PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. I updated the patch so it does not warn about 'A * B << C'. It's a simple fix. I have not made careful measurements but I guess that the performance penalty is acceptable. https://reviews.llvm.org/D24861 ___

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 72802. danielmarjamaki added a comment. Fix review comments https://reviews.llvm.org/D24656 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantDeclarationCheck.cpp

Re: [PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 3 inline comments as done. Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:39 @@ +38,3 @@ + bool MultiVar = false; + if (const auto *VD = dyn_cast(D)) { +if (VD && VD->getPreviousDecl()->getStorageClass() == SC_Extern &&

Re: [PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-09-27 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. Compiling 2064 projects resulted in 904 warnings Here are the results: https://drive.google.com/file/d/0BykPmWrCOxt2N04tYl8zVHA3MXc/view?usp=sharing The results looks acceptable imho. The code looks intentional in many cases so I believe there are users that

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked an inline comment as done. Comment at: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp:195 @@ +194,3 @@ +if (Optional S = I->getAs()) { + if (!isa(S->getStmt())) +return S->getStmt(); yes I agree.

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 72775. danielmarjamaki added a comment. Use !isa. Suggestion by Gabor. https://reviews.llvm.org/D24905 Files: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-09-28 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. https://reviews.llvm.org/D24905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

2016-10-03 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki closed this revision. danielmarjamaki added a comment. r283095 https://reviews.llvm.org/D24759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24905: Fix unreachable code false positive, vardecl in switch

2016-10-03 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D24905#557573, @dcoughlin wrote: > Sorry, missed this patch. > > I think it would good to add a test to make sure we do warn when the var decl > has an initializer, since that will not be executed. > > void varDecl(int X) { >

SV: [PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-10-03 Thread Daniel Marjamäki via cfe-commits
Hello! Moving it to a subgroup such as -Wparentheses is fine for me. I will look at that when I have time. Do you think this warning has an acceptable output then? Best regards, Daniel Marjamäki

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

2016-09-26 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/Analysis/CFG.cpp:2986 @@ -2985,3 +2985,1 @@ -if (!KnownVal.isFalse()) { - // Add an intermediate block between the BodyBlock and the dcoughlin wrote: > You need to keep this check so that the

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

2016-09-26 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 72461. danielmarjamaki marked 2 inline comments as done. danielmarjamaki added a comment. Fixed review comments by Devin. It works better now! https://reviews.llvm.org/D24759 Files: lib/Analysis/CFG.cpp test/Analysis/unreachable-code-path.c

[PATCH] D25606: alpha.core.UnreachableCode - don't warn about unreachable code inside macro

2016-10-14 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added reviewers: NoQ, dcoughlin. danielmarjamaki added subscribers: cfe-commits, xazax.hun, zaks.anna, a.sidorin. danielmarjamaki set the repository for this revision to rL LLVM. This patch fixes false positives for such code: #define

[PATCH] D25596: alpha.core.Conversion - Fix false positive for 'U32 += S16; ' expression, that is not unsafe

2016-10-14 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added a reviewer: NoQ. danielmarjamaki added subscribers: cfe-commits, xazax.hun, dcoughlin. danielmarjamaki set the repository for this revision to rL LLVM. This patch fix false positives for loss of sign in addition and subtraction

[PATCH] D25606: alpha.core.UnreachableCode - don't warn about unreachable code inside macro

2016-10-18 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL284477: alpha.core.UnreachableCode - don't warn about unreachable code inside macro (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D25606?vs=74849=74990#toc

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-10-18 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. Ping. Repository: rL LLVM https://reviews.llvm.org/D24656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26911: readability-redundant-declarations: Fix crash

2016-11-21 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL287540: readability-redundant-declaration: Fix crash (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D26911?vs=78706=78715#toc Repository: rL LLVM

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-11-01 Thread Daniel Marjamäki via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL285689: [clang-tidy] Add check readability-redundant-declaration (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D24656?vs=74478=76548#toc Repository: rL LLVM

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-10-12 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added inline comments. Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:60 +auto Diag = diag(D->getLocation(), "redundant '%0' declaration") +<< cast(D)->getName(); +if (!MultiVar && !DifferentHeaders) alexfh

[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

2016-10-13 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki set the repository for this revision to rL LLVM. danielmarjamaki updated this revision to Diff 74478. danielmarjamaki added a comment. Herald added a subscriber: modocache. changed cast(D)->getName() to cast(D) Repository: rL LLVM https://reviews.llvm.org/D24656 Files:

[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

2016-10-13 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D24861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-13 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. I agree with the comments from you dcoughlin but I am not sure how to do it. > Can you also add a test that tests this more directly (i.e., with > clang_analyzer_warnIfReached). I don't think it is good to have the only test > for this core coverage issue to be

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

2016-10-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D25326#564584, @zaks.anna wrote: > Please, fix the style issues before committing. Sorry I missed that. Ideally it would be possible to run clang-format on the files before committing. but currently I get lots of unrelated changes

[PATCH] D25606: alpha.core.UnreachableCode - don't warn about unreachable code inside macro

2016-10-17 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 74849. danielmarjamaki added a comment. make pattern to avoid warnings more specific Repository: rL LLVM https://reviews.llvm.org/D25606 Files: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp test/Analysis/unreachable-code-path.c

<    1   2   3   >