vsavchenko marked an inline comment as done.
vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:459-469
+ if (Origin.From().isMinSignedValue()) {
+ // If mini is a minimal signed value, absolute value of it is greater
+ // than the maximal signed value. In order to avoid these
+ // complications, we simply return the whole range.
+ return {ValueFactory.getMinValue(RangeType),
+ ValueFactory.getMaxValue(RangeType)};
+ }
----------------
vsavchenko wrote:
> ASDenysPetrov wrote:
> > I think you should swap `if` statements. I'll explain.
> > Let's consider the input is an **uint8** range [42, 242] and you will
> > return [0, 242] in the second `if`.
> > But if the input is an **uint8** range [128, 242] you will return [0, 255]
> > in the first `if`, because 128 is an equivalent of -128(INT8_MIN) in binary
> > representation so the condition in the first if would be true.
> > What is the great difference between [42, 242] and [128, 242] to have
> > different results? Or you've just missed this case?
> >
> > P.S. I think your function's name doesn't fit its body, since //absolute
> > value// is always positive (without sign) from its definition, but you
> > output range may have negative values. You'd better write an explanation
> > above the function and rename it.
> It is a valid point, I will add this test and change this behaviour!
>
> The name is confusing indeed, maybe you have any ideas what would be more
> appropriate?
@NoQ , @ASDenysPetrov what do you think about this name instead (i.e.
`getSymmetricalRange`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80117/new/
https://reviews.llvm.org/D80117
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
