[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 27 20:11:17 PDT 2018
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
More information about the cfe-commits
mailing list