Charusso 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;
----------------
NoQ wrote:
> 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.
Oh, fail. Thanks!
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:507
+ ConstraintRangeTy Constraints = State->get<ConstraintRange>();
+ for (const SymbolRef CurrentSym : SIE->symbols()) {
+ if (CurrentSym == SIE)
----------------
NoQ wrote:
> 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.
It is rather sequential in the bitwise world, but I think if you would look
only at the LHS `SymExpr`, it should be enough. Also I would like to avoid
extra overhead, that `symbols()` business was large enough. Thanks!
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
