public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Mikael Morin <morin-mikael@orange.fr>
To: Harald Anlauf <anlauf@gmx.de>, fortran@gcc.gnu.org
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, v3] Fortran: ordering of hidden procedure arguments [PR107441]
Date: Tue, 8 Nov 2022 11:32:17 +0100	[thread overview]
Message-ID: <54c4f997-863d-f850-ddf9-1ed780feedff@orange.fr> (raw)
In-Reply-To: <24c6acfa-6745-c7a3-4bbd-54bd0fa31454@gmx.de>

Hello,

Le 07/11/2022 à 22:45, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> Am 04.11.22 um 10:53 schrieb Mikael Morin:
>> Le 03/11/2022 à 23:03, Harald Anlauf a écrit :
>>> I've spent some time not only staring at create_function_arglist,
>>> but trying several variations handling the declared hidden parms,
>>> and applying the necessary adjustments to gfc_get_function_type.
>>> (Managing linked trees is not the issue, just understanding them.)
>>> I've been unable to get the declarations in sync, and would need
>>> help how to debug the mess I've created.  Dropping my patch for
>>> the time being.
>>>
>> If you want, we can meet on IRC somewhen (tonight?).
> 
> armed with the new knowledge, I could now understand what
> (more or less) trivially went wrong with my previous patch.
> 
> The attached patch remedies that: gfc_get_function_type() now
> properly separates the types of the hidden parameters so that
> optional+value comes before string length and caf stuff,
> while in create_function_arglist we simply need to split
> the walking over the typelists so that the optional+value
> stuff, which is basically just booleans, is done separately
> from the other parts.
> 
> Looking at the tree-dumps, the function decls now seem to be
> fine at least for the given testcases.  I've adjusted one of
> the testcases to validate this.
> 
> Regtests fine on x86_64-pc-linux-gnu.  OK for mainline?
> 
this is mostly good.
There is one last corner case that is not properly handled:

> diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
> index 63515b9072a..94988b8690e 100644
> --- a/gcc/fortran/trans-decl.cc
> +++ b/gcc/fortran/trans-decl.cc
(...)
> @@ -2619,6 +2620,15 @@ create_function_arglist (gfc_symbol * sym)
>      if (f->sym != NULL)	/* Ignore alternate returns.  */
>        hidden_typelist = TREE_CHAIN (hidden_typelist);
>  
> +  /* Advance hidden_typelist over optional+value argument presence flags.  */
> +  optval_typelist = hidden_typelist;
> +  for (f = gfc_sym_get_dummy_args (sym); f; f = f->next)
> +    if (f->sym != NULL
> +	&& f->sym->attr.optional && f->sym->attr.value
> +	&& !f->sym->attr.dimension && f->sym->ts.type != BT_CLASS
> +	&& !gfc_bt_struct (f->sym->ts.type))
> +      hidden_typelist = TREE_CHAIN (hidden_typelist);
> +

This new loop copies the condition guarding the handling of optional 
value presence arguments, except that the condition is in an "else if", 
and the complement of the condition in the corresponding "if" is 
missing, to have strictly the same conditions.

Admittedly, it only makes a difference for character optional value 
arguments, which are hardly working.  At least they work as long as one 
doesn't try to query their presence.  Below is a case regressing with 
your patch.

With that fixed, I think it's good for mainline.
Thanks for your patience.


! { dg-do compile }
!
! PR fortran/107441
! Check that procedure types and procedure decls match when the procedure
! has both character-typed and character-typed optional value args.
!
! Contributed by M.Morin

program p
   interface
     subroutine i(c, o)
       character(*) :: c
       character(3), optional, value :: o
     end subroutine i
   end interface
   procedure(i), pointer :: pp
   pp => s
   call pp("abcd", "xyz")
contains
   subroutine s(c, o)
     character(*) :: c
     character(3), optional, value :: o
     if (o /= "xyz") stop 1
     if (c /= "abcd") stop 2
   end subroutine s
end program p



  reply	other threads:[~2022-11-08 10:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-28 20:12 [PATCH] " Harald Anlauf
2022-10-30 19:23 ` Mikael Morin
2022-10-30 20:32   ` Mikael Morin
2022-10-30 21:25   ` Mikael Morin
2022-10-31  9:57     ` Mikael Morin
2022-10-31 20:29       ` [PATCH, v2] " Harald Anlauf
2022-11-02 17:20         ` Mikael Morin
2022-11-02 21:19           ` Harald Anlauf
2022-11-03 10:06             ` Mikael Morin
2022-11-03 22:03               ` Harald Anlauf
2022-11-04  9:53                 ` Mikael Morin
2022-11-07 21:45                   ` [PATCH, v3] " Harald Anlauf
2022-11-08 10:32                     ` Mikael Morin [this message]
2022-11-08 20:31                       ` Harald Anlauf
2022-11-08 21:39                         ` Mikael Morin

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=54c4f997-863d-f850-ddf9-1ed780feedff@orange.fr \
    --to=morin-mikael@orange.fr \
    --cc=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    /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).