[lldb-dev] Signedness of scalars built from APInt(s)

[Jonas Devlieghere via lldb-dev]

If I understand the situation correctly I think we should do both. I'd
start by doing (2) to improve the current behavior and add a constructor
for APSInt. We can then audit the call sites and migrate to APSInt where
it's obvious that the type is signed. That should match the semantics of
both classes?

On Fri, Jan 4, 2019 at 2:00 PM Davide Italiano <dccitaliano at gmail.com>
wrote:

> On Fri, Jan 4, 2019 at 1:57 PM Davide Italiano <dccitaliano at gmail.com>
> wrote:
> >
> > While adding support for 512-bit integers in `Scalar`, I figured I
> > could add some coverage.
> >
> > TEST(ScalarTest, Signedness) {
> >  auto s1 = Scalar(APInt(32, 12, false /* isSigned */));
> >  auto s2 = Scalar(APInt(32, 12, true /* isSigned */ ));
> >  ASSERT_EQ(s1.GetType(), Scalar::e_uint); // fails
> >  ASSERT_EQ(s2.GetType(), Scalar::e_sint); // pass
> > }
> >
> > The result of `s1.GetType()` is Scalar::e_sint.
> > This is because an APInt can't distinguish between "int patatino = 12"
> > and "uint patatino = 12".
> > The correct class in `llvm` to do that is `APSInt`.
> >
>
> Please note that this is also broken in the case where you have
> APInt(32 /* bitWidth */, -323);
> because of the way the constructor is implemented.
>
> --
> Davide
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190104/21bb9855/attachment.html>