NoQ added a comment.
LGTM! Minor nitpicking in comments.
Currently there's no such problem, but as `GetRange` becomes more complicated,
we'll really miss the possibility of saying something like "if there's a range
for negated symbol, `return GetRange(the negated symbol)`", so that all other
special cases applied. We could have allowed that by canonicalizing symbols
(i.e. announce that `$A` always goes before `$B` and therefore we will store a
range for `$A - $B` even if we're told to store the range for `$B - $A`) and
then the "if" will become "if the symbol is not canonical, `return GetRange(the
canonicalized symbol)`".
================
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:546
+ if (const SymSymExpr *SSE = dyn_cast<SymSymExpr>(Sym)) {
+ // FIXME: Once SValBuilder supports unary minus, we should use SValBuilder
----------------
Can we move the whole `if` into a function?
Eg.,
```
if (RangeSet *R = getRangeForMinusSymbol(Sym))
return R->Negate(BV, F)
```
?
================
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:557
+ SSE->getLHS(), T);
+ if (ConstraintRangeTy::data_type *negV =
+ State->get<ConstraintRange>(negSym)) {
----------------
`ConstraintRangeTy::data_type` ~> `RangeSet` should be easier to read.
https://reviews.llvm.org/D35110
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
