steakhal added a comment.
This patch seems pretty straightforward, good job @manas, and for the folks
giving review comments.
Aside from polishing the tests, I think it's good to go.
================
Comment at: clang/test/Analysis/constant-folding.c:470
+
+void testEqualityRules(unsigned int a, unsigned int b, int c, int d) {
+ // Checks when ranges are not overlapping
----------------
================
Comment at: clang/test/Analysis/constant-folding.c:470
+
+void testEqualityRules(unsigned int a, unsigned int b, int c, int d) {
+ // Checks when ranges are not overlapping
----------------
steakhal wrote:
>
I would prefer `u1`, `u2`, `s1`, `s2` respectively.
This way the name would signify the signess of the variable.
================
Comment at: clang/test/Analysis/constant-folding.c:473
+ if (a <= 10 && b >= 20) {
+ clang_analyzer_eval((a != b) != 0); // expected-warning{{TRUE}}
+ }
----------------
Isn't `a != b` enough? Why do you need the `(..) != 0` part?
================
Comment at: clang/test/Analysis/constant-folding.c:476-478
+ if (c <= INT_MIN + 10 && d >= INT_MAX - 10) {
+ clang_analyzer_eval((c != d) == 0); // expected-warning{{FALSE}}
+ }
----------------
How is this different compared to the previous case? The difference I can see
is that now we use different constants and the `==` operator in the outer
expression. None of which should really change the behavior AFAICT.
================
Comment at: clang/test/Analysis/constant-folding.c:481
+ // Checks when ranges are completely overlapping and have more than one point
+ if (a >= 20 && a <= 50 && b >= 20 && b <= 50) {
+ clang_analyzer_eval((a != b) != 0); // expected-warning{{UNKNOWN}}
----------------
You could have a comment that `a: [20,50] b:[20,50]`.
It would be easier to comprehend than the chain of conjunctions.
Similarly, how at L464 does.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106102/new/
https://reviews.llvm.org/D106102
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
