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

[Greg Clayton via lldb-dev]

Option 3? Add APSInt as a new member?

Current:

  Scalar::Type m_type;
  llvm::APInt m_integer;
  llvm::APFloat m_float;
  bool m_ieee_quad = false;

Option #3:

  Scalar::Type m_type;
  llvm::APInt m_uint;
  llvm::APSInt m_sint;
  llvm::APFloat m_float;
  bool m_ieee_quad = false;


I don't know enough about APInt and APSInt to know if one or the other can correctly be used for mixed operations.

> On 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`.
> 
> I think there are at least a couple of possibilities to fix this:
> 1) Change the constructor in Scalar to always get an APSInt. This
> would be fairly invasive but we could always query isSigned() to make
> sure we instantiate the correct scalar type.
> 2) Change the implementation of Scalar(APInt v) to look at the sign
> bit, and if that's set, treat the value as signed (and unsigned
> otherwise). This will be definitely more correct than the current
> implementation which always construct signed types from APInt(s), but
> I'm not entirely sure of all the implications.
> 
> What do you think?
> 
> --
> Davide

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190104/fca42283/attachment.html>