public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Burnus, Tobias" <Tobias_Burnus@mentor.com>
To: "Loosemore, Sandra" <Sandra_Loosemore@mentor.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	fortran <fortran@gcc.gnu.org>,
	Paul Richard Thomas <paul.richard.thomas@gmail.com>
Subject: *Ping* [Patch] Fortran: Fix bind(C) character length checks
Date: Thu, 8 Jul 2021 05:14:14 +0000	[thread overview]
Message-ID: <fa2601b45224489697501991b8f4ab4b@svr-ies-mbx-01.mgc.mentorg.com> (raw)
In-Reply-To: <16bdad19-5548-f271-e7b2-997632bf29e1@codesourcery.com>

*Ping*

I intent to incorporate Sandra's suggestions, except for the beginning of line spacing - that's needed to avoid exceeding the 80 character line limit. I did not include an updated patch as just pinging is easier on a mobile during vacation :-)

Thanks,

Tobias

Loosemore, Sandra wrote:

On 7/1/21 11:08 AM, Tobias Burnus wrote:
> Hi all,
>
> this patch came up when discussing Sandra's TS29113 patch internally.
> There is presumably also some overlap with José's patches.
>
> This patch tries to rectify the BIND(C) CHARACTER handling on the
> diagnostic side, only. That is: what to accept and what
> to reject for which Fortran standard.
>
>
> The rules are:
>
> * [F2003-F2018] Interoperable is character(len=1)
>    → F2018, 18.3.1  Interoperability of intrinsic types
>    (General, unchanged)
>
> * Fortran 2008: In some cases, const-length chars are
>    permitted as well:
>    → F2018, 18.3.4  Interoperability of scalar variables
>    → F2018, 18.3.5  Interoperability of array variables
>    → F2018, 18.3.6  Interoperability of procedures and procedure interfaces
>   [= F2008, 15.3.{4,5,6}
> For global vars with bind(C), 18.3.4 + 18.3.5 applies directly (TODO:
> Add support, not in this patch)
> For passed-by ref dummy arguments, 18.3.4 + 18.3.5 are referenced in
> - F2008: R1229  proc-language-binding-spec is language-binding-spec
>           C1255 (R1229) <see wording there>
> - F2018, F2018, C1554
>
> While it is not very clearly spelt out, I regard 'char parm[4]'
> interoperable with 'character(len=4) :: a', 'character(len=2) :: b(2)'
> and 'character(len=1) :: c(4)' for both global variables and for
> dummy arguments.
>
> * Fortran 2018/TS29113:  Uses additionally CFI array descriptor
>    - allocatable, pointer:  must be len=:
>    - nonallocatable/nonpointer: len=* → implies array descriptor also
>      for assumed-size/explicit-size/scalar arguments.
>    - All which all passed by an array descriptor already without further
>      restrictions: assumed-shape, assumed-rank, i.e. len=<nonconst> seems
>      to be also fine
> → 18.3.6 under item (5) bullet point 2 and 3 plus (6).
>
>
> I hope I got the conditions right. I also fixed an issue with
> character(len=5) :: str – the code in trans-expr.c did crash for
> scalars  (decl.c did not check any constraints for arrays).
> I believe the condition is wrong and for len=<const> no descriptor
> is used.
>
> Any comments, remarks?

I gave this patch a try on my TS 29113 last night.  Changing the error
messages kind of screwed up my list of FAILs, but I did see that it also
caught some invalid character arguments in
interoperability/typecodes-scalar.f90 and
interoperability/typecodes-scalar-ext.f90 (which are already broken by 2
other major gfortran bugs I still need to file PRs for).  :-S

I haven't tried to review the patch WRT correctness with the
requirements of the standard yet, but I have a few nits about error
messages....

> +               /* F2018, 18.3.6 (6).  */
> +               if (!sym->ts.deferred)
> +                 {
> +                   gfc_error ("Allocatable and pointer character dummy "
> +                              "argument %qs at %L must have deferred length "
> +                              "as procedure %qs is BIND(C)", sym->name,
> +                              &sym->declared_at, sym->ns->proc_name->name);
> +                   retval = false;
> +                 }

This is the error the two aforementioned test cases started giving, but
message is confusing and doesn't read well (it was a pointer dummy, not
"allocatable and pointer").  Maybe just s/and/or/, or customize the
message depending on which one it is?

> +                   gfc_error ("Character dummy argument %qs at %L must be "
> +                              "of constant length or assumed length, "
> +                              "unless it has assumed-shape or assumed-rank, "
> +                              "as procedure %qs has the BIND(C) attribute",
> +                              sym->name, &sym->declared_at,
> +                              sym->ns->proc_name->name);

I don't think either "assumed-shape" or "assumed-rank" should be
hyphenated in this context unless that exact hyphenation is a term of
art in the Fortran standard or other technical documentation.  In normal
English, adjective phrases are usually only hyphenated when they appear
immediately before the noun they modify; "assumed-shape array", but "an
array with assumed shape".

> +               else if (!gfc_notify_std (GFC_STD_F2018,
> +                                         "Character dummy argument %qs at %L"
> +                                         " with nonconstant length as "
> +                                         "procedure %qs is BIND(C)",
> +                                         sym->name, &sym->declared_at,
> +                                         sym->ns->proc_name->name))
> +                 retval = false;
> +             }

Elsewhere the convention seems to be to format strings split across
multiple lines with a space at the end of each chunk rather than at the
beginning.

-Sandra
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

  reply	other threads:[~2021-07-08  5:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 17:08 Tobias Burnus
2021-07-01 19:30 ` Sandra Loosemore
2021-07-02 22:13 ` Sandra Loosemore
2021-07-08  5:14   ` Burnus, Tobias [this message]
2021-07-14 12:50     ` *Ping**2 " Burnus, Tobias
2021-07-16 12:55       ` Jerry D
2021-07-21  9:24         ` Tobias Burnus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa2601b45224489697501991b8f4ab4b@svr-ies-mbx-01.mgc.mentorg.com \
    --to=tobias_burnus@mentor.com \
    --cc=Sandra_Loosemore@mentor.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=paul.richard.thomas@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).