On Thu, Dec 13, 2018 at 12:26:50AM +0000, Ramsay Jones wrote:
> On 12/12/2018 16:45, Luc Van Oostenryck wrote:
> > static void predefined_max(const char *name, const char *suffix, unsigned bits)
> > {
> > - unsigned long long max = bits_mask(bits - 1);
> > + unsigned long long max = bits_mask(bits);
> > char buf[32];
>
> Hmm, this can't possibly be right, unless ...
...
> > + if (flags & PTYPE_MAX) {
> > + const char *suffix = builtin_type_suffix(type);
> > + predefined_max(name, suffix, bits - is_signed_type(type));
>
> ... you subtract one from the bits parameter for a signed type!
>
> Hmm, I'm not sure I like this. I had expected something like:
>
> static void predefined_max(const char *name, struct symbol *type)
> {
> unsigned bits = type->bit_size;
> const char *suffix = builtin_type_suffix(type);
> unsigned long long max;
> char buf[32];
>
> if (is_signed_type(type))
> bits = bits - 1;
> max = bits_mask(bits);
> snprintf(buf, sizeof(buf), "__%s_MAX__", name);
> predefine(buf, 1, "%#llx%s", max, suffix);
> }
>
> ... or something like that. (but it is your call ;-) ).
>
Absolutely, but this can't be done at in this patch because
the predefined_max() with an explicit number of bits is still
needed for wchar which receive a type only in the next patch
(and I switching the patch order bring others complications
with wchar).
I'll fix it in one of the subsequent patches.
-- Luc
> BTW, I haven't finished testing (something else came up), but
> some platforms are using the 'long' type for wchar_t. (Also, I
> suspect some of the tests won't work ...) Oh, also, there seems
> to be a discrepancy in the size of 'long double'.
>
> [I will try to finish testing tomorrow]
>
> Thanks!
Thanks to you for the testing!
For the platforms using long for wchar_t, yes, it's the same kind
of mess as for int32_t. I really don't like these #ifdeferies
(like currently done for int32_t). Of course, having the default
correct for the native platform is the minimal but ...
It's really a nasty problem since there is a lot of flags
that can modify these types (I'm thinking at -mlong-double-64
on x86, -mabi=... on ARM & MIPS, ...) plus the fact that it
is OS dependent.
-- Luc
