[PATCH 05/11] fortran: array descriptor: Add accessors for the type field

[Tobias Burnus]

Mikael Morin wrote:
> Add accessor functions to get or set the value of the type field of array
> descriptors, and remove from the public API the function giving direct acces
> to the field.

This is one is interesting: The setter not only comes as set of
overloaded functions that modify a stmtblock_t block (taking a
tree or an int), but also one variant that returns a TREE,
returning the modification via gfc_finish_block.

* * *

> +static tree
> +get_type_field (tree type, unsigned field_idx)
> +{
> +  tree field = gfc_advance_chain (TYPE_FIELDS (type), field_idx);
> +  gcc_assert (field != NULL_TREE);
> +
> +  return field;
> +}

If you already introduce such a function just to add
the assert (and the TYPE_FIELDS macro), why don't you
remove the assert at:

>   gfc_get_descriptor_field (tree desc, unsigned field_idx)
>   {
>     tree type = TREE_TYPE (desc);
>     gcc_assert (GFC_DESCRIPTOR_TYPE_P (type));
>   
> -  tree field = gfc_advance_chain (TYPE_FIELDS (type), field_idx);
> +  tree field = get_type_field (type, field_idx);
>     gcc_assert (field != NULL_TREE);
and
> +gfc_conv_descriptor_type_set (stmtblock_t *block, tree desc, int value)
> +{
> +  tree type = TREE_TYPE (desc);
> +  gcc_assert (GFC_DESCRIPTOR_TYPE_P (type));
> +
> +  tree dtype = get_type_field (type, DTYPE_FIELD);
> +  gcc_assert (dtype != NULL_TREE);
> +
> +  tree field = get_type_field (TREE_TYPE (dtype), GFC_DTYPE_TYPE);
> +  gcc_assert (field != NULL_TREE);
* * *


>
> gcc/fortran/ChangeLog:
>
> 	* trans-descriptor.cc (get_type_field): New function.
> 	(gfc_get_descriptor_field): Use it.
> 	(gfc_conv_descriptor_type): Make static and rename ...
> 	(conv_descriptor_type): ... to this.
> 	(gfc_conv_descriptor_type_get, gfc_conv_descriptor_type_set): New
> 	functions.
> 	* trans-descriptor.h (gfc_conv_descriptor_type): Remove declaration.
> 	(gfc_conv_descriptor_type_get, gfc_conv_descriptor_type_set): New
> 	declarations.
> 	* trans-expr.cc (gfc_conv_gfc_desc_to_cfi_desc): Use
> 	gfc_conv_descriptor_type_get to get the value of the type field.
> 	* trans-decl.cc (gfc_conv_cfi_to_gfc): Use
> 	gfc_conv_descriptor_type_set to set the value of the type field.
* * *
> +++ b/gcc/fortran/trans-descriptor.cc
> @@ -61,13 +61,28 @@ along with GCC; see the file COPYING3.  If not see
>   #define LBOUND_SUBFIELD 1
>   #define UBOUND_SUBFIELD 2
>   
> +
> +/* Get FIELD_IDX'th field in struct TYPE.  */
> +
> +static tree
> +get_type_field (tree type, unsigned field_idx)
> +{
> +  tree field = gfc_advance_chain (TYPE_FIELDS (type), field_idx);
> +  gcc_assert (field != NULL_TREE);
> +
> +  return field;
> +}
> +
> +
> +/* Get FIELD_IDX'th field in array descriptor DESC.  */
> +
>   static tree
>   gfc_get_descriptor_field (tree desc, unsigned field_idx)
>   {
...
> +  tree field = get_type_field (type, field_idx);
...
>     return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),

First, I think a *the* is missing for "Get (the) FIELD_IDX-th field".

But I think the comment does not really make clear the difference: The first
one gets the declaration of the struct field. The second one gets the component
ref to a field. Maybe use for the second one:

"Return component ref to the FIELD_IDX-th field of array descriptor DESC." ?

* * *

> +/* Return some code setting to VALUE the type discriminator of the array
> +   descriptor DESC.  */
...
> +gfc_conv_descriptor_type_set (tree desc, tree value)
...
> +  return gfc_finish_block (&block);
> +}

"Return some code" sounds odd. How about: "Return modify expr ..."?Or less explicit: "Return code block ...".

(gfc_finish_block = "return block->head", if not empty and not has_scope,
if pre/post had to do something, it uses append_to_statement_list to create
a sequence.)

* * *

Otherwise, LGTM.

Tobias