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

Reply via email to