public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Paul Richard Thomas <paul.richard.thomas@gmail.com>
To: Harald Anlauf <anlauf@gmx.de>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	trnka@scm.com
Subject: Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab
Date: Sun, 31 Mar 2024 12:08:38 +0000	[thread overview]
Message-ID: <CAGkQGiJHrsEKR9JKGP6GyAso3pSD1+KR4k4nQqGaYWvoruMuCw@mail.gmail.com> (raw)
In-Reply-To: <c5538067-194c-404f-9661-20586d7e0f64@gmx.de>


[-- Attachment #1.1: Type: text/plain, Size: 1729 bytes --]

Hi Harald,

>
> I had only a quick glance at your patch.  I guess you unintentionally
> forgot to remove those parts that you already committed for PR110987,
> along with the finalize-testcases.
>

Guilty as charged. I guess I got out of the wrong side of the bed :-)

>
> I am still trying to find the precise paragraph in the standard
> you refer to regarding INTENT(OUT) and default initialization.
>

Page 114 of the draft F2023 standard:
"The INTENT (OUT) attribute for a nonpointer dummy argument specifies that
the dummy argument becomes undefined on invocation of the procedure, except
for any subcomponents that are default-initialized (7.5.4.6)."
With the fix, gfortran behaves in the same way as ifort and nagfor.

On rereading the patch, I think that s/"and use the passed value"/"and
leave undefined"/ or some such is in order.


> While at it, I think I found a minor nit in testcase pr112407a.f90:
> component x%i appears undefined the first time it is printed.
>

Fixed - thanks for pointing it out.

A correct patch is attached.

Thanks for looking at the previous, overloaded version.

Paul



>
> > 2024-03-30  Paul Thomas  <pault@gcc.gnu.org>
> >
> > gcc/fortran
> > PR fortran/112407
> > *resolve.cc (resolve_procedure_expression): Change the test for
> > for recursion in the case of hidden procedures from modules.
> > (resolve_typebound_static): Add warning for possible recursive
> > calls to typebound procedures.
> > * trans-expr.cc (gfc_trans_class_init_assign): Do not apply
> > default initializer to class dummy where component initializers
> > are all null.
> >
> > gcc/testsuite/
> > PR fortran/112407
> > * gfortran.dg/pr112407a.f90: New test.
> > * gfortran.dg/pr112407b.f90: New test.
> >
>
>

[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch, Size: 6231 bytes --]

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 50d51b06c92..43315a6a550 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1963,12 +1963,20 @@ resolve_procedure_expression (gfc_expr* expr)
       || (sym->attr.function && sym->result == sym))
     return true;

-  /* A non-RECURSIVE procedure that is used as procedure expression within its
+   /* A non-RECURSIVE procedure that is used as procedure expression within its
      own body is in danger of being called recursively.  */
   if (is_illegal_recursion (sym, gfc_current_ns))
-    gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
-		 " itself recursively.  Declare it RECURSIVE or use"
-		 " %<-frecursive%>", sym->name, &expr->where);
+    {
+      if (sym->attr.use_assoc && expr->symtree->name[0] == '@')
+	gfc_warning (0, "Non-RECURSIVE procedure %qs from module %qs is "
+		     " possibly calling itself recursively in procedure %qs. "
+		     " Declare it RECURSIVE or use %<-frecursive%>",
+		     sym->name, sym->module, gfc_current_ns->proc_name->name);
+      else
+	gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		     " itself recursively.  Declare it RECURSIVE or use"
+		     " %<-frecursive%>", sym->name, &expr->where);
+    }

   return true;
 }
@@ -6820,6 +6828,13 @@ resolve_typebound_static (gfc_expr* e, gfc_symtree** target,
       if (st)
 	*target = st;
     }
+
+  if (is_illegal_recursion ((*target)->n.sym, gfc_current_ns)
+      && !e->value.compcall.tbp->deferred)
+    gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		 " itself recursively.  Declare it RECURSIVE or use"
+		 " %<-frecursive%>", (*target)->n.sym->name, &e->where);
+
   return true;
 }

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 76bed9830c4..f3fcba2bd59 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1719,6 +1719,7 @@ gfc_trans_class_init_assign (gfc_code *code)
   tree tmp;
   gfc_se dst,src,memsz;
   gfc_expr *lhs, *rhs, *sz;
+  gfc_component *cmp;

   gfc_start_block (&block);

@@ -1735,6 +1736,21 @@ gfc_trans_class_init_assign (gfc_code *code)
   /* The _def_init is always scalar.  */
   rhs->rank = 0;

+  /* Check def_init for initializers.  If this is a dummy with all default
+     initializer components NULL, return NULL_TREE and use the passed value as
+     required by F2018(8.5.10).  */
+  if (!lhs->ref && lhs->symtree->n.sym->attr.dummy)
+    {
+      cmp = rhs->ref->next->u.c.component->ts.u.derived->components;
+      for (; cmp; cmp = cmp->next)
+	{
+	  if (cmp->initializer)
+	    break;
+	  else if (!cmp->next)
+	    return build_empty_stmt (input_location);
+	}
+    }
+
   if (code->expr1->ts.type == BT_CLASS
       && CLASS_DATA (code->expr1)->attr.dimension)
     {
diff --git a/gcc/testsuite/gfortran.dg/pr112407a.f90 b/gcc/testsuite/gfortran.dg/pr112407a.f90
new file mode 100644
index 00000000000..470f4191611
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr112407a.f90
@@ -0,0 +1,71 @@
+! { dg-do run }
+! Test of an issue found in the investigation of PR112407
+! Contributed by Tomas Trnka  <trnka@scm.com>
+!
+module m
+  private new_t
+
+  type s
+    procedure(),pointer,nopass :: op
+  end type
+
+  type :: t
+    integer :: i
+    type (s) :: s
+  contains
+    procedure :: new_t
+    procedure :: bar
+    procedure :: add_t
+    generic :: new => new_t, bar
+    generic, public :: assignment(=) => add_t
+    final :: final_t
+  end type
+
+  integer :: i = 0, finals = 0
+
+contains
+  recursive subroutine new_t (arg1, arg2)
+    class(t), intent(out) :: arg1
+    type(t), intent(in)  :: arg2
+    i = i + 1
+
+    print "(a,2i4)", "new_t", arg1%i, arg2%i
+    if (i .ge. 10) return
+
+! According to F2018(8.5.10), arg1 should be undefined on invocation, unless
+! any sub-components are default initialised. gfc used to set arg1%i = 0.
+    if (arg1%i .ne. arg2%i) then
+      arg1%i = arg2%i
+      call arg1%new(arg2)
+    endif
+  end
+
+  subroutine bar(arg)
+    class(t), intent(out) :: arg
+    call arg%new(t(42, s(new_t)))
+  end
+
+  subroutine add_t (arg1, arg2)
+    class(t), intent(out) :: arg1
+    type(t), intent(in)  :: arg2
+    call arg1%new (arg2)
+  end
+
+  impure elemental subroutine final_t (arg1)
+    type(t), intent(in) :: arg1
+    finals = finals + 1
+  end
+end
+
+  use m
+  class(t), allocatable :: x
+  allocate(x)
+  x%i = 0
+  call x%new()                   ! gfortran used to output 10*'new_t'
+  print "(3i4)", x%i, i, finals  !           -||-          0 10 11
+!
+! The other brands output 2*'new_t' + 42 2 3 and now so does gfc :-)
+  if (x%i .ne. 42) stop 1
+  if (i .ne. 2) stop 2
+  if (finals .ne. 3) stop 3
+end
diff --git a/gcc/testsuite/gfortran.dg/pr112407b.f90 b/gcc/testsuite/gfortran.dg/pr112407b.f90
new file mode 100644
index 00000000000..e541825d616
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr112407b.f90
@@ -0,0 +1,56 @@
+! { dg-do compile }
+! Test of an issue found in the investigation of PR112407
+! Contributed by Tomas Trnka  <trnka@scm.com>
+!
+module m
+  private new_t
+
+  type s
+    procedure(),pointer,nopass :: op
+  end type
+
+  type :: t
+    integer :: i
+    type (s) :: s
+  contains
+    procedure :: new_t
+    procedure :: bar
+    procedure :: add_t
+    generic :: new => new_t, bar
+    generic, public :: assignment(=) => add_t
+    final :: final_t
+  end type
+
+  integer :: i = 0, finals = 0
+
+contains
+  subroutine new_t (arg1, arg2)            ! gfortran didn't detect the recursion
+    class(t), intent(out) :: arg1
+    type(t), intent(in)  :: arg2
+    i = i + 1
+
+    print *, "new_t", arg1%i, arg2%i
+    if (i .ge. 10) return
+
+    if (arg1%i .ne. arg2%i) then
+      arg1%i = arg2%i
+      call arg1%new(arg2)  ! { dg-warning "possibly calling itself recursively" }
+    endif
+  end
+
+  subroutine bar(arg)
+    class(t), intent(out) :: arg
+    call arg%new(t(42, s(new_t)))
+  end
+
+  subroutine add_t (arg1, arg2)
+    class(t), intent(out) :: arg1
+    type(t), intent(in)  :: arg2
+    call arg1%new (arg2)
+  end
+
+  impure elemental subroutine final_t (arg1)
+    type(t), intent(in) :: arg1
+    finals = finals + 1
+  end
+end

  reply	other threads:[~2024-03-31 12:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-30  9:06 Paul Richard Thomas
2024-03-30 14:52 ` Harald Anlauf
2024-03-31 12:08   ` Paul Richard Thomas [this message]
2024-04-01 20:04     ` Harald Anlauf

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=CAGkQGiJHrsEKR9JKGP6GyAso3pSD1+KR4k4nQqGaYWvoruMuCw@mail.gmail.com \
    --to=paul.richard.thomas@gmail.com \
    --cc=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=trnka@scm.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).