This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] support static nested constructors in bitfields (1/2)
- From: Ian Lance Taylor <iant at google dot com>
- To: Olivier Hainque <hainque at adacore dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 26 May 2009 07:33:09 -0700
- Subject: Re: [PATCH] support static nested constructors in bitfields (1/2)
- References: <20090427103204.GA29582@cardhu.act-europe.fr>
Olivier Hainque <hainque@adacore.com> writes:
> 2009-04-27 Olivier Hainque <hainque@adacore.com>
>
> * varasm.c (oc_local_state): New structure, output_constructor
> local state to support communication with helpers.
> (output_constructor_array_range): New output_constructor helper,
> extracted code for an array range element.
> (output_constructor_regular_field): New output_constructor helper,
> extracted code for an element that is not a bitfield.
> (output_constructor_bitfield): New output_constructor helper,
> extracted code for a bitfield element.
> (output_constructor): Rework to use helpers.
> + /* Datastructures and helpers for output_constructor. */
> +
> + /* Local output_constructor state to support interaction with helpers. */
> +
> + typedef struct {
> +
> + /* Received arguments. */
> + tree exp; /* constructor expression. */
> + unsigned HOST_WIDE_INT size; /* # bytes to output - pad if necessary. */
> + unsigned int align; /* known initial alignment. */
> +
> + /* Constructor expression data. */
> + tree type; /* expression type. */
> + tree field; /* current field decl in a record. */
> + tree min_index; /* lower bound if specified for an array. */
> +
> + /* Output processing state per se. */
> + HOST_WIDE_INT total_bytes; /* # bytes output so far / current position. */
> + int byte_buffer_in_use; /* whether byte ... */
> + int byte; /* ... contains part of a bitfield byte yet to
> + be output. */
> +
> + /* Current element. */
> + tree val; /* current element value. */
> + tree index; /* current element index. */
> +
> + } oc_local_state;
All comment sentences should start with a capital letter.
> + static void output_constructor_array_range (oc_local_state *);
> + static void output_constructor_regular_field (oc_local_state *);
> + static void output_constructor_bitfield (oc_local_state *);
Current gcc style is to omit these static declarations unless they are
required for circularity. Please omit if possible.
> + if (local->val == 0)
Test against NULL_TREE, not 0 (I know the existing code uses 0).
> + if (local->val != 0 && TREE_CODE (local->val) != INTEGER_CST)
> + error ("invalid initial value for member %qs",
> + IDENTIFIER_POINTER (DECL_NAME (local->field)));
Test against NULL_TREE, not 0. Also, it seems that if you don't return,
or change local->var here, you are going to get useless follow-on
errors. You should probably do one or the other unless you know that
can't happen.
> + if (local->val == 0)
> + local->val = integer_zero_node;
Test against NULL_TREE, not 0.
> + local.field = 0;
s/0/NULL_TREE/
> ! /* For a field that is not a bitfield. */
> ! else if (local.field == 0 || !DECL_BIT_FIELD (local.field))
> ! output_constructor_regular_field (&local);
s/0/NULL_TREE/
This patch is OK with those changes.
Thanks.
Ian