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

2016-09-20 Thread Anna Zaks via cfe-commits
zaks.anna added inline comments. Comment at: test/Analysis/ReturnNonBoolTest.c:67 @@ +66,3 @@ + + if (rc < 0) +// error handling How about addressing this as follows: in checkBranchCondition, you check for any comparisons of the tracked value other than comp

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

2016-09-20 Thread Anton Urusov via cfe-commits
urusant added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ReturnNonBoolChecker.cpp:50 @@ +49,3 @@ + if (!State->contains(SR)) return; + + ExplodedNode *N = C.generateErrorNode(C.getState()); I have just noticed that I didn't specify the style option

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

2016-09-20 Thread Anton Urusov via cfe-commits
urusant updated this revision to Diff 71927. Repository: rL LLVM https://reviews.llvm.org/D24507 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/StaticAnalyzer/Checkers/Checkers.td lib/Sema/SemaChecking.cpp

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] D24507: Add attribute for return values that shouldn't be cast to bool

2016-09-20 Thread Anton Urusov via cfe-commits
urusant updated this revision to Diff 71921. urusant added a comment. In https://reviews.llvm.org/D24507#546380, @aaron.ballman wrote: > We try to keep our tests segregated by functionality. e.g., tests relating to > the way the attribute is handled (what it appertains to, args, etc) should > l

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

2016-09-19 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. In https://reviews.llvm.org/D24507#546241, @urusant wrote: > Thank you for the feedback. > > > The patch is missing Sema tests for the attribute (that it only applies to > > declarations you expect, accepts no args, etc). > > > There is one test case for that in te

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

2016-09-19 Thread Anton Urusov via cfe-commits
urusant added a comment. Thank you for the feedback. > The patch is missing Sema tests for the attribute (that it only applies to > declarations you expect, accepts no args, etc). There is one test case for that in test/ReturnNonBoolTestCompileTime.cpp. I've added another one for attribute ac

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

2016-09-19 Thread Anton Urusov via cfe-commits
urusant updated this revision to Diff 71807. urusant added a comment. Made some changes based on the comments. Please refer to the replies below. Repository: rL LLVM https://reviews.llvm.org/D24507 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/D

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

2016-09-15 Thread Anna Zaks via cfe-commits
zaks.anna added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2055 @@ -2054,1 +2054,3 @@ } +def WarnImpcastToBoolDocs : Documentation { + let Category = DocCatFunction; You probably need to "propose" the attribute to the clang community. I'd send

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

2016-09-15 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. Thank you for working on this check! A few comments: The patch is missing Sema tests for the attribute (that it only applies to declarations you expect, accepts no args, etc). Have you considered making this a type attribute on the return type of the function rat