Re: [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long

[Joel Brobecker <brobecker@adacore.com>]

Hi Tom,

On Thu, Feb 22, 2018 at 01:30:15PM -0700, Tom Tromey wrote:
> unpack_bits_as_long is documented as sign-extending its result when
> the type is signed.  However, it was only doing sign-extension in the
> case where the field was a bitfield -- that is, not when the "bitsize"
> parameter was 0, indicating the size should be taken from the type.
> 
> Also, unpack_bits_as_long was incorrectly computing the shift for
> big-endian architectures for the non-bitfield case.
> 
> This patch fixes these bugs in a straightforward way.  A new selftest
> is included.
> 
> 2018-02-22  Tom Tromey  <tom@tromey.com>
> 
> 	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> 	unittests/unpack-selftests.c.
> 	* unittests/unpack-selftests.c: New file.
> 	* value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases.

Looks good to me. Just one thing: I think we need to adjust
unpack_bits_as_long's documentation a bit to match the reality of
how this function is called:

    /* Unpack a bitfield of the specified FIELD_TYPE, from the object at
       VALADDR, and store the result in *RESULT.
       The bitfield starts at BITPOS bits and contains BITSIZE bits.

I checked the callers, and none of them really make a distinction between
bitsize = 0 and bitsize != 0. And since it's fairly straightforward to
handlet this case directly in unpack_bits_as_long, I agree this is best.

Pre-approved with this change.
Thanks for adding a self-test!

-- 
Joel