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

gcc/fortran/ChangeLog:

        * trans-descriptor.cc (gfc_conv_descriptor_elem_len): Make static
        and rename ...
        (conv_descriptor_elem_len): ... to this.
        (gfc_conv_descriptor_elem_len_get,
        gfc_conv_descriptor_elem_len_set): New functions.
        * trans-descriptor.h (gfc_conv_descriptor_elem_len): Remove
        declaration.
        (gfc_conv_descriptor_elem_len_get,
        gfc_conv_descriptor_elem_len_set): New declarations.
        * trans-decl.cc (gfc_conv_cfi_to_gfc): Use
        gfc_conv_descriptor_elem_len_get to get the value of the elem_len
        field and gfc_conv_descriptor_elem_len_set to set it.
        * trans-array.cc (gfc_array_init_size,
        gfc_alloc_allocatable_for_assignment): Likewise.
        * trans-expr.cc (gfc_conv_scalar_to_descriptor,
        gfc_conv_gfc_desc_to_cfi_desc, gfc_trans_pointer_assignment):
        Likewise.
        * trans-intrinsic.cc (gfc_conv_is_contiguous_expr,
        gfc_conv_intrinsic_sizeof): Likewise.
        * trans-openmp.cc (gfc_omp_array_size, gfc_omp_deep_mapping_item):
        Likewise.
* * *
--- a/gcc/fortran/trans-descriptor.cc
+++ b/gcc/fortran/trans-descriptor.cc
...
+/* Return the descriptor DESC's array element size in bytes.  */
+
+tree
+gfc_conv_descriptor_elem_len_get (tree desc)
+{
+  return conv_descriptor_elem_len (desc);
+}
+
+/* Add code to BLOCK setting to VALUE the descriptor DESC's size (in bytes) of
+   array elements.  */
+
+void
+gfc_conv_descriptor_elem_len_set (stmtblock_t *block, tree desc, tree value)

I find the wording of the comment not very elegant and also
misleading ("DESC's size of array elements"?).

How about:

"Add code to BLOCK to set the descriptor DESC's field for the size (in bytes) of
an array element to VALUE'."

or

"Add code to BLOCK for setting DESC's elem_len to VALUE, the size (in bytes)
of an array element."

Or something like that.

Otherwise, the patch looks good to me (LGTM/OK) – and as noted in the email to
Part 1, especially the following is a nice cleanup:

-      tmp = gfc_conv_descriptor_dtype (tmp);
-      field = gfc_advance_chain (TYPE_FIELDS (get_dtype_type_node ()),
-                                GFC_DTYPE_ELEM_LEN);
-      tmp = fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
-                            tmp, field, NULL_TREE);
+      tmp = gfc_conv_descriptor_elem_len_get (tmp);
(Even if it could have been done also using 'gfc_conv_descriptor_elem_len' 
before.)

Thanks for the patch!

Tobias

Reply via email to