[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

[Balázs Benics via Phabricator via cfe-commits]

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