On Tue, Sep 9, 2014 at 6:02 AM, Dominik Vogt <v...@linux.vnet.ibm.com> wrote:
> This patch extends the -fdump-go-spec option to handle bitfields
> and unions and fixes handlinx of zero length arrays. All of this
> is necessary for s390[x] since the system headers use these
> features. Please check the commit comment for a detailed
> description of the patch.
>
> gcc/ChangeLog
> -------------
> 2014-09-05 Dominik Vogt <v...@linux.vnet.ibm.com>
>
> * godump.c (precision_to_units): New helper function.
> (go_append_artificial_name): Ditto.
> (go_append_decl_name): Ditto.
> (go_append_bitfield): Ditto.
> (go_get_uinttype_for_precision): Ditto.
> (go_append_padding): Ditto.
> (go_force_record_alignment): Ditto.
> (go_format_type): Represent unions with an array of uints of the size
> of the alignment in go. This fixes the 'random' size of the union's
> representation using just the first field.
> (go_format_type): Add argument that indicates whether a record is
> nested (used for generation of artificial go names).
> (go_output_fndecl): Adapt to new go_format_type signature.
> (go_output_typedef): Ditto.
> (go_output_var): Ditto.
> (go_output_var): Prefer to output type as alias (typedef).
> (go_format_type): Bitfields in records are simulated as arrays of
> bytes
> in go.
>
> 2014-09-05 Dominik Vogt <v...@linux.vnet.ibm.com>
>
> * godump.c (go_format_type): Fix handling of arrays with zero
> elements.
>
> gcc/testsuite/ChangeLog
> -----------------------
> 2014-09-05 Dominik Vogt <v...@linux.vnet.ibm.com>
>
> * gcc.misc-tests/godump-1.c: Add tests for bitfields and unions.
>
> 2014-09-05 Dominik Vogt <v...@linux.vnet.ibm.com>
>
> * gcc.misc-tests/godump.exp: New.
> * gcc.misc-tests/godump-1.c: New.
> + case 8:
> + return (is_unsigned) ? "uint8" : "int8";
No need to parenthesize is_unsigned here and in the following lines.
> +#if 1 /*!!!todo: identifier may not be unique???*/
> +#endif
I'm not sure what the #if 1 is about. Just put a FIXME comment--see
other examples in the code base.
> + static unsigned int art_i = 0;
>
> + if (is_nested == false)
> + art_i = 0;
Don't use a static variable. Instead, pass down an int*. It can be
NULL for the case where !is_nested. (Also, in code like this, write
!is_nested rather than is_nested == false).
> + layout_type (type);
Is this really necessary? At this point the types should be laid out
already.
+ decl_align_unit = precision_to_units (DECL_ALIGN (field));
You can just use DECL_ALIGN_UNIT.
> + type_align_unit = precision_to_units (TYPE_ALIGN (type));
You can just use TYPE_ALIGN_UNIT.
> + snprintf (buf, sizeof buf, "%u", sz_units);
This doesn't look right--shouldn't it be
sz_units / TYPE_ALIGN_UNITS (type)
? Using sz_units only looks right if go_get_uinttype_for_precision
returns byte.
> + ( !htab_find_slot (container->type_hash, IDENTIFIER_POINTER (id),
Don't use these leading spaces after parens, just let emacs format
your code even if looks awkward.
Thanks.
Ian
