vsavchenko marked an inline comment as done.
vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:556
RangeSet infer(SymbolRef Sym) {
- const RangeSet *AssociatedRange = State->get<ConstraintRange>(Sym);
-
- // If Sym is a difference of symbols A - B, then maybe we have range set
- // stored for B - A.
- const RangeSet *RangeAssociatedWithNegatedSym =
- getRangeForMinusSymbol(State, Sym);
-
- // If we have range set stored for both A - B and B - A then calculate the
- // effective range set by intersecting the range set for A - B and the
- // negated range set of B - A.
- if (AssociatedRange && RangeAssociatedWithNegatedSym)
- return AssociatedRange->Intersect(
- ValueFactory, RangeFactory,
- RangeAssociatedWithNegatedSym->Negate(ValueFactory, RangeFactory));
-
- if (AssociatedRange)
- return *AssociatedRange;
-
- if (RangeAssociatedWithNegatedSym)
- return RangeAssociatedWithNegatedSym->Negate(ValueFactory, RangeFactory);
+ if (Optional<RangeSet> ConstraintBasedRange = intersect(
+ ValueFactory, RangeFactory, State->get<ConstraintRange>(Sym),
----------------
NoQ wrote:
> I'm a bit confused, why aren't these invocations of
> `getRangeForInvertedSub()` and `getRangeForComparisonSymbol()` implemented
> simply as steps in `Visit()`? What's so different about them, other then our
> desire not to go into infinite recursion (which should probably be addressed
> via memoization)?
I even had a patch that moves these two functions inside of the `Visit`, but
then I reverted that change.
Right now the logic for each symbol is like this: let's try to find anything on
this symbol in the existing constraints and if we did find something - return
it. And only if that didn't work, we try to figure out something from the
sub-tree on its own.
So what does that mean in here: that we will need to make a performance
impacting [probably nothing serious, but still a functional] change - to
intersect the range assigned to the symbol and whatever we can infer from its
subtrees. It would be good to have when we support binary operators
`+/-/==/<=/etc.` because it can help narrowing it down.
As of now, it will give no information, only possible overhead of traversing a
tree. For this reason, I suggest to move those separately and AFTER we support
more operators, because that will also have a couple of good motivation
examples.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82381/new/
https://reviews.llvm.org/D82381
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
