[PATCH] D140086: [analyzer][solver] Improve reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1642
+    if (LHS.isUnsigned() ^ RHS.isUnsigned()) {
+      if (LHS.isUnsigned() && (LHS.getBitWidth() >= RHS.getBitWidth())) {
+
----------------
steakhal wrote:
> manas wrote:
> > steakhal wrote:
> > > Why do we need this additional condition?
> > > If I remove these, I get no test failures, which suggests to me that we have some undertested code paths here.
> > > Why do we need this additional condition?
> > 
> > Bitwidth was important because we should ideally cast smaller bitwidth type to bigger bitwidth type.
> > 
> > Consider if we have `LHS(u8), RHS(i32)`, then without checking for bitwidth, we would be casting RHS's maxValue to LHS's type, which will result in lose of information and will not serve our purpose.
> > 
> > 
> If you think we need that bitwidth check, why did you remove it?
> I'd like to see test cases demonstrating what we are talking about and see if we want that behavior or not.
This test fails.

```

void testfoo(unsigned char u, signed int s) {
  if (u >= 253 && u <= 255 && s < INT_MAX - 2) {
    // u: [253, 254], s: [INT_MIN, INT_MAX - 2]
    clang_analyzer_eval(u != s); // expected-warning{{UNKNOWN}}
                                 // but returns TRUE
  }
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140086/new/

https://reviews.llvm.org/D140086