NoQ added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:496
+  // as a bitwise operation result could be null.
+  if (RS.getConcreteValue() && RS.getConcreteValue()->getExtValue() == 0)
+    return State;
----------------
Instead of "we know that the value is null", we should write "we //don't// know 
that the value is //non-//null". I.e. if we're not sure, we must still do an 
early return.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:507
+  ConstraintRangeTy Constraints = State->get<ConstraintRange>();
+  for (const SymbolRef CurrentSym : SIE->symbols()) {
+    if (CurrentSym == SIE)
----------------
This loop doesn't make sense. When your expression looks like `(((a + b) + c) + 
d) & (e + f)`, you don't want to iterate separately over `a`, `b`, `c`, `d`, 
`e`, `f`; but that's what this loop would do. You should only look at the LHS 
and the RHS. If you want to descend further, do so recursively, so that to keep 
track of the overall structure of the symbolic expression as you're traversing 
it, rather than traversing sub-expressions in an essentially random order.


================
Comment at: clang/test/Analysis/bitwise-ranges.cpp:21
+
+  // CHECK: { "symbol": "reg_$0<unsigned int X>", "range": "{ [2, 3] }" }
+  // CHECK: { "symbol": "(reg_$0<unsigned int X>) & 1U", "range": "{ [1, 1] }" 
}
----------------
This test accidentally tests debug prints. We don't want to test debug prints 
here. I suggest:
```lang=c++
clang_analyzer_eval(X >= 2 && X <= 3); // expected-warning{{TRUE}}
```


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

https://reviews.llvm.org/D65239



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to