NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Math looks good to me :)
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:388
+ /// it will return the range [x_0, y_N].
+ static Range roughen(RangeSet Origin) {
+ assert(!Origin.isEmpty());
----------------
vsavchenko wrote:
> xazax.hun wrote:
> > Is `roughen` part of a nomenclature? If not, what about something like
> > `cover`?
> Yeah 😆 I spend quite some time thinking about a proper name for that one. It
> is used to be `coarsen` (yep, yuck!), but I don't really feel "cover" either.
> Mb something like "unify" will work? What do you think?
Well, this is basically a one-dimensional //convex hull//...
But i do think that something like `fillGaps()` would be easier to understand.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:418
+
+ auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+ auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);
----------------
vsavchenko wrote:
> xazax.hun wrote:
> > Why do we need this conversion?
> > Do we want to model a cast in the code somewhere?
> > If so, I think this is more like a workaround and in the future we would
> > need an explicit way to represent those cast operations. It might be worth
> > to have a TODO.
> Those ranges that we inferred have nothing to do with upper operations. I
> mean, in theory, `VisitBinaryOperator` can take care of conversions, but I
> don't know how it can be done in the most generic way. In this particular
> situation, we really care that `From` and `To` don't change signs or
> something like that after conversion, but for other operators it might be not
> so important.
>
> Answering the question of why we even need those, putting it simple - there
> is no other way, this is how `APSInt` works, couldn't do anything with values
> of different types (e.g. comparison).
>
> I am totally on board with you on the inconvenience of the whole thing.
> Every implementer of the `Visit*` method should take care of conversions on
> their own. I would say that `SValBuilder` adding special symbolic values for
> this kind of casts, can be a solution.
Yeah i think this is correct. This is exactly how our symbolic expressions
aren't type-safe: expression of type T corresponds to result of operations
during which operand symbols were promoted to T.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79336/new/
https://reviews.llvm.org/D79336
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
