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
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
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
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
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
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
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
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
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
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
10 matches
Mail list logo