ASDenysPetrov added inline comments.
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:1130
+ // Check inverting single range.
+ this->checkInvert({{MIN, MAX}}, {});
+ this->checkInvert({{MIN, MID}}, {{MID + 1, MAX}});
----------------
martong wrote:
> ASDenysPetrov wrote:
> > martong wrote:
> > > I'd expect that inversion on finite sets is an invert-able function thus
> > > ```
> > > this->checkInvert({}, {MIN, MAX});
> > > ```
> > > would make sense instead of assertion.
> > >
> > > Besides, it would make sense to test the composition of two invert
> > > operations to identity.
> > I'm afraid the function would be overheaded and less usable. Why do we have
> > an assertion here? I thought about this. This is kinda a tradeoff.
> >
> > What range would be a correct range from inversion of `[]`? `[-128, 127]`?
> > `[0, 65535]`?
> > In order to make `[MIN, MAX]` from empty range `[]` we should know the
> > exact `APSIntType`. We extract the type from the given range
> > `What.getAPSIntType();`.
> >
> > But though, let's consider we have `QualType` as a second parameter. We
> > have two options about what type to use.
> > 1. Always use the second parameter. The function would not only invert but
> > do what `castTo` actually do. Such behavior would violates a single
> > responsibility principle and duplicate the functionality.
> > 2. Always use the type from the given range and for the case of empty range
> > from the second parameter. The function contract would be more complex and
> > obscure.
> >
> > So, I decided not to introduce a new parameter and shift responsibility to
> > the user. Empty range is an edge case when we usually produce infeasible
> > state or use `infer(QualType)`. This may pretty fit with what user is going
> > to do first before using `invert`, so it shouldn't affect the convinience
> > of function usage too much.
> Okay, I see your point.
>
> So, considering `RangeSet::Factory::invert(RangeSet What)` we have only one
> parameter and that is the RangeSet to be inverted. And that set might be
> empty and in that case we are left without any type information, thus there
> is no way to return [MIN, MAX]. Please correct me if I am wrong.
Exactly. That's what I'm talking about..
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130372/new/
https://reviews.llvm.org/D130372
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
