Re: [PATCH 1/3] [PR libfortran/101305] Bind(C): Fix type encodings in ISO_Fortran_binding.h

[Tobias Burnus <tobias@codesourcery.com>]

On 13.07.21 23:28, Sandra Loosemore wrote:

> ISO_Fortran_binding.h had many incorrect hardwired kind encodings in
> the definitions of the CFI_type_* macros.  Additionally, not all
> targets support all the defined type encodings, and the Fortran
> standard requires those macros to have a negative value.
>
> This patch changes ISO_Fortran_binding.h to use sizeof instead of
> hard-coded sizes, and assembles it from fragments that reflect the
> set of types supported by the target.
>
> 2021-07-13  Sandra Loosemore<sandra@codesourcery.com>
>           Tobias Burnus<tobias@codesourcery.com>
>
> libgfortran/
>       PR libfortran/101305
>       * ISO_Fortran_binding.h: Fix hard-coded sizes and split into...
>       * ISO_Fortran_binding-1-tmpl.h: New file.
>       * ISO_Fortran_binding-2-tmpl.h: New file.
>       * ISO_Fortran_binding-3-tmpl.h: New file.
>       * Makefile.am: Add rule for generating ISO_Fortran_binding.h.
>       Adjust pathnames to that file.
>       * Makefile.in: Regenerated.
>       * mk-kinds-h.sh: New file.
>       * runtime/ISO_Fortran_binding.c: Fix include path.
LGTM – except for the following remark regarding a preexisting comment.

> --- /dev/null
> +++ b/libgfortran/ISO_Fortran_binding-1-tmpl.h
> +/* Error codes.
> +   CFI_INVALID_STRIDE should be defined in the standard because they are useful to the implementation of the functions.
> + */

The standard permits: "Error conditions other than those listed in this
subclause should be indicated by error codes different from the values
of the macros named in this subclause."

I personally do not like current (preexisting) the wording in the
comment – and CFI_FAILURE is also not listed, which is also not part
of Fortran standard. I think some wording along the following is
be more appropriate:
"Note that CFI_FAILURE and CFI_INVALID_STRIDE specific to GCC and not
part of the Fortran standard."


> +#define CFI_SUCCESS 0
> +#define CFI_FAILURE 1
> +#define CFI_ERROR_BASE_ADDR_NULL 2
> +#define CFI_ERROR_BASE_ADDR_NOT_NULL 3
> +#define CFI_INVALID_ELEM_LEN 4
> +#define CFI_INVALID_RANK 5
> +#define CFI_INVALID_TYPE 6
> +#define CFI_INVALID_ATTRIBUTE 7
> +#define CFI_INVALID_EXTENT 8
> +#define CFI_INVALID_STRIDE 9
> +#define CFI_INVALID_DESCRIPTOR 10
> +#define CFI_ERROR_MEM_ALLOCATION 11
> +#define CFI_ERROR_OUT_OF_BOUNDS 12

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955