Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Sandra Loosemore]

On 10/2/21 2:28 PM, Harald Anlauf wrote:

Hi Tobias,

the corrected attached patch fixes the regression for testcase
default_initialization_3.f90 for me now, and as a bonus matches
the description.


Me too!  I'm also seeing clean test results now.

-Sandra

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Harald Anlauf via Gcc-patches]

Hi Tobias,

the corrected attached patch fixes the regression for testcase
default_initialization_3.f90 for me now, and as a bonus matches
the description.

Am 02.10.21 um 21:56 schrieb Tobias Burnus:

Hi Harald,

unfortunately, your email did not arrive at fort...@gcc.gnu.org – nor at
my private address.

I copied it from
https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580794.html

You wrote:

>/I do not see this error. Can you double check that you indeed use
the />/posted patch:
/>//>/https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html

– />/and nothing else, e.g., an earlier draft? /
Sandra is right.  Actually I do see that regression, too.
The default initialization is missing in F1, see dump-tree:

Look as if I had attached the first/interim version of the patch –
which lacked what I wrote in the patch submission at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html

Namely, I wrote:

Regarding the patch, '!= IFSRC_IFBODY' has to be used; "== IFSRC_DECL"
won't work as the the generatedy ENTRY master function has IFSRC_UNKNOWN.

Thus, no surprise that it passes here – while you see the fail.

Tobias



Harald

[I had trouble posting via gmane lately, and I haven't found out
yet what is causing it.]

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Tobias Burnus]

Hi Harald,

unfortunately, your email did not arrive at fort...@gcc.gnu.org – nor at my 
private address.

I copied it from 
https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580794.html

You wrote:
>/I do not see this error. Can you double check that you indeed use the />/posted patch: />//>/https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html 
 – />/and nothing else, e.g., an earlier draft? /

Sandra is right.  Actually I do see that regression, too.
The default initialization is missing in F1, see dump-tree:

Look as if I had attached the first/interim version of the patch –
which lacked what I wrote in the patch submission at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html

Namely, I wrote:

Regarding the patch, '!= IFSRC_IFBODY' has to be used; "== IFSRC_DECL"
won't work as the the generatedy ENTRY master function has IFSRC_UNKNOWN.

Thus, no surprise that it passes here – while you see the fail.

Tobias

Fortran: Avoid var initialization in interfaces [PR54753]

Intent(out) implies deallocation/default initialization; however, it is
pointless to do this for dummy-arguments symbols of procedures which are
inside an INTERFACE block. – This also fixes a bogus error for the attached
included testcase, but fixing the non-interface version still has to be done.

	PR fortran/54753

gcc/fortran/ChangeLog:

	* resolve.c (can_generate_init, resolve_fl_variable_derived,
	resolve_symbol): Only do initialization with intent(out) if not
	inside of an interface block.

gcc/testsuite/ChangeLog:

	* gfortran.dg/assumed_rank_23.f90: New test.

 gcc/fortran/resolve.c | 11 ---
 gcc/testsuite/gfortran.dg/assumed_rank_23.f90 | 16 
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 30b96b2f597..511fe3a5e55 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12676,7 +12676,8 @@ can_generate_init (gfc_symbol *sym)
 || a->cray_pointer
 || sym->assoc
 || (!a->referenced && !a->result)
-|| (a->dummy && a->intent != INTENT_OUT)
+|| (a->dummy && (a->intent != INTENT_OUT
+		 || sym->ns->proc_name->attr.if_source == IFSRC_IFBODY))
 || (a->function && sym != sym->result)
   );
 }
@@ -12913,7 +12914,9 @@ resolve_fl_variable_derived (gfc_symbol *sym, int no_init_flag)
 
   /* Assign default initializer.  */
   if (!(sym->value || sym->attr.pointer || sym->attr.allocatable)
-  && (!no_init_flag || sym->attr.intent == INTENT_OUT))
+  && (!no_init_flag
+	  || (sym->attr.intent == INTENT_OUT
+	  && sym->ns->proc_name->attr.if_source != IFSRC_IFBODY)))
 sym->value = gfc_generate_initializer (>ts, can_generate_init (sym));
 
   return true;
@@ -16154,7 +16157,8 @@ resolve_symbol (gfc_symbol *sym)
 		|| sym->ts.u.derived->attr.alloc_comp
 		|| sym->ts.u.derived->attr.pointer_comp))
 	   && !(a->function && sym != sym->result))
-	  || (a->dummy && a->intent == INTENT_OUT && !a->pointer))
+	  || (a->dummy && !a->pointer && a->intent == INTENT_OUT
+	  && sym->ns->proc_name->attr.if_source != IFSRC_IFBODY))
 	apply_default_init (sym);
   else if (a->function && sym->result && a->access != ACCESS_PRIVATE
 	   && (sym->ts.u.derived->attr.alloc_comp
@@ -16166,6 +16170,7 @@ resolve_symbol (gfc_symbol *sym)
 
   if (sym->ts.type == BT_CLASS && sym->ns == gfc_current_ns
   && sym->attr.dummy && sym->attr.intent == INTENT_OUT
+  && sym->ns->proc_name->attr.if_source != IFSRC_IFBODY
   && !CLASS_DATA (sym)->attr.class_pointer
   && !CLASS_DATA (sym)->attr.allocatable)
 apply_default_init (sym);
diff --git a/gcc/testsuite/gfortran.dg/assumed_rank_23.f90 b/gcc/testsuite/gfortran.dg/assumed_rank_23.f90
new file mode 100644
index 000..c83aa7de1a3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/assumed_rank_23.f90
@@ -0,0 +1,16 @@
+! { dg-do compile }
+!
+! PR fortran/54753
+! TS29113:C535c
+! F2018:C839
+!
+module m
+
+  interface
+subroutine s1 (x, y)
+  class(*) :: x(..)
+  class(*), intent (out) :: y(..)
+end subroutine
+  end interface
+
+end module 

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Harald Anlauf via Gcc-patches]

Hi Tobias,

Am 02.10.21 um 20:29 schrieb Tobias Burnus:

On 02.10.21 20:01, Sandra Loosemore wrote:

On 9/29/21 2:53 AM, Tobias Burnus wrote:

There are three issues, this patch solves the first:
* reject-valid issue due to adding the initializer also to a dummy
   argument which is in an INTERFACE block. Having initializers in
   INTERFACE blocks is pointless and causes for the attached testcase

   the bogus error:

   "Assumed-rank variable y at (1) may only be used as actual argument"
...
This has indeed allowed me to make progress on adding the diagnostic, 
but I'm seeing some test regressions on x86_64-linux-gnu that are due 
to this patch alone:


FAIL: gfortran.dg/default_initialization_3.f90   -O0  execution test


I do not see this error. Can you double check that you indeed use the 
posted patch:


https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html – 
and nothing else, e.g., an earlier draft?


Sandra is right.  Actually I do see that regression, too.
The default initialization is missing in F1, see dump-tree:

__attribute__((fn spec (". R w w ")))
integer(kind=4) master.0.f1 (integer(kind=8) __entry, struct t1 * d2, 
struct t1 * d1)

{
  integer(kind=4) res;
  integer(kind=4) __result_master.0.f1;
  integer(kind=4) res.0 [value-expr: __result_master.0.f1];

  if (d1 != 0B)
{
  {
struct t1 t1.1;

t1.1.i = 7;
*d1 = t1.1;
  }
}

The latter "if (d1 != 0B)" block gets removed by your change, and
this is not good.

(I still have to commit it; it was approved by Harald, but he also he 
preferred to have a second view, I decided to wait until tomorrow/Monday 
before committing it.)


Well, I did not regtest, only read the patch and trust that you did your
homework!


Tobias


Harald

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Tobias Burnus]

On 02.10.21 20:01, Sandra Loosemore wrote:

On 9/29/21 2:53 AM, Tobias Burnus wrote:

There are three issues, this patch solves the first:
* reject-valid issue due to adding the initializer also to a dummy
   argument which is in an INTERFACE block. Having initializers in
   INTERFACE blocks is pointless and causes for the attached testcase
   the bogus error:
   "Assumed-rank variable y at (1) may only be used as actual argument"
...
This has indeed allowed me to make progress on adding the diagnostic, 
but I'm seeing some test regressions on x86_64-linux-gnu that are due 
to this patch alone:


FAIL: gfortran.dg/default_initialization_3.f90   -O0  execution test


I do not see this error. Can you double check that you indeed use the 
posted patch:


https://gcc.gnu.org/pipermail/gcc-patches/2021-September/580483.html – 
and nothing else, e.g., an earlier draft?


(I still have to commit it; it was approved by Harald, but he also he 
preferred to have a second view, I decided to wait until tomorrow/Monday 
before committing it.)


Tobias

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Sandra Loosemore]

On 9/29/21 2:53 AM, Tobias Burnus wrote:

Found when looking at F2018:C839 / PR54753.

For INTENT(OUT) the dummy variable (might) also be default initialized
or deallocated. However, with assumed rank, that causes issues, which
C839 prevents. In the current GCC implementation, missing C839 constraint
diagnostic, but also rejects-valid/ice-on-valid appears.

There are three issues, this patch solves the first:

* reject-valid issue due to adding the initializer also to a dummy
   argument which is in an INTERFACE block. Having initializers in
   INTERFACE blocks is pointless and causes for the attached testcase
   the bogus error:
   "Assumed-rank variable y at (1) may only be used as actual argument"

(Except for wasting resources and this error, they should be ignored
in trans*.c and usually do not cause any further harm.)


I think Sandra has a nearly ready patch to do the C839 constraint
diagnostic, which needs the attached patch to do the checks.

The third issue is that GCC currently gives either an ICE or the
above error message when declaring a procedure with a valid
assumed-rank intent(out) dummy. This has still to be solved as well.
But first I wanted to unblock Sandra's C839 work with this patch :-)


This has indeed allowed me to make progress on adding the diagnostic, 
but I'm seeing some test regressions on x86_64-linux-gnu that are due to 
this patch alone:


FAIL: gfortran.dg/default_initialization_3.f90   -O0  execution test
FAIL: gfortran.dg/default_initialization_3.f90   -O1  execution test
FAIL: gfortran.dg/default_initialization_3.f90   -O2  execution test
FAIL: gfortran.dg/default_initialization_3.f90   -O3 
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer 
-finline-functions  execution test

FAIL: gfortran.dg/default_initialization_3.f90   -O3 -g  execution test
FAIL: gfortran.dg/default_initialization_3.f90   -Os  execution test

It is failing with STOP 5, so I guess it is over-eagerly removing 
initialization in something that is not an interface.


BTW, I think it would be better to open a new issue for this and the 
"third issue" you describe for this than to tag them with PR54753, since 
these initialization bugs aren't really related to the missing 
diagnostic in that issue except so far as the bogus error/crashes make 
it impossible to test for the missing diagnostic.  Personally, I find it 
really confusing to track which bugs have really been fixed when 
multiple bugs that need different fixes are lumped together in the same 
issue...  there might be patches committed for such a mashed-up issue, 
but which parts have been fixed and which not, and when is it safe to 
close the issue?  :-S


-Sandra

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Harald Anlauf via Gcc-patches]

[Resending as I did not see it show up in the MLs]

Hi Tobias,

Am 29.09.21 um 10:53 schrieb Tobias Burnus:

Found when looking at F2018:C839 / PR54753.

For INTENT(OUT) the dummy variable (might) also be default initialized
or deallocated. However, with assumed rank, that causes issues, which
C839 prevents. In the current GCC implementation, missing C839 constraint
diagnostic, but also rejects-valid/ice-on-valid appears.

There are three issues, this patch solves the first:

* reject-valid issue due to adding the initializer also to a dummy
   argument which is in an INTERFACE block. Having initializers in
   INTERFACE blocks is pointless and causes for the attached testcase
   the bogus error:
   "Assumed-rank variable y at (1) may only be used as actual argument"


ACK.


(Except for wasting resources and this error, they should be ignored
in trans*.c and usually do not cause any further harm.)


I think Sandra has a nearly ready patch to do the C839 constraint
diagnostic, which needs the attached patch to do the checks.

The third issue is that GCC currently gives either an ICE or the
above error message when declaring a procedure with a valid
assumed-rank intent(out) dummy. This has still to be solved as well.
But first I wanted to unblock Sandra's C839 work with this patch :-)


Regarding the patch, '!= IFSRC_IFBODY' has to be used; "== IFSRC_DECL"
won't work as the the generatedy ENTRY master function has IFSRC_UNKNOWN.


I have to admit that the code touched is hard to understand for me.
The conditions involved are unfortunately already dense, long lists.

(There are PRs related to *missing* default initializations (e.g.
PR100440), which I looked at before in that area until I got lost...
Of course this needs to be addressed elsewhere.)


OK for mainline?


No objections here; the patch seems to work.  The commit message has
a non-ASCII character (hyphen).  Not sure whether that was intended.

You may nevertheless want a second opinion.

Thanks for the patch!

Harald


Tobias

PS: Some patch reviews are that fast that it is impossible to send the OK;
at least, I did not manage to do for Harald's last two - for the last one
I was at least 4min too late. ;-)

-
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

Re: [Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Harald Anlauf via Gcc-patches]

Hi Tobias,

Am 29.09.21 um 10:53 schrieb Tobias Burnus:

Found when looking at F2018:C839 / PR54753.

For INTENT(OUT) the dummy variable (might) also be default initialized
or deallocated. However, with assumed rank, that causes issues, which
C839 prevents. In the current GCC implementation, missing C839 constraint
diagnostic, but also rejects-valid/ice-on-valid appears.

There are three issues, this patch solves the first:

* reject-valid issue due to adding the initializer also to a dummy
   argument which is in an INTERFACE block. Having initializers in
   INTERFACE blocks is pointless and causes for the attached testcase
   the bogus error:
   "Assumed-rank variable y at (1) may only be used as actual argument"


ACK.


(Except for wasting resources and this error, they should be ignored
in trans*.c and usually do not cause any further harm.)


I think Sandra has a nearly ready patch to do the C839 constraint
diagnostic, which needs the attached patch to do the checks.

The third issue is that GCC currently gives either an ICE or the
above error message when declaring a procedure with a valid
assumed-rank intent(out) dummy. This has still to be solved as well.
But first I wanted to unblock Sandra's C839 work with this patch :-)


Regarding the patch, '!= IFSRC_IFBODY' has to be used; "== IFSRC_DECL"
won't work as the the generatedy ENTRY master function has IFSRC_UNKNOWN.


I have to admit that the code touched is hard to understand for me.
The conditions involved are unfortunately already dense, long lists.

(There are PRs related to *missing* default initializations (e.g.
PR100440), which I looked at before in that area until I got lost...
Of course this needs to be addressed elsewhere.)


OK for mainline?


No objections here; the patch seems to work.  The commit message has
a non-ASCII character (hyphen).  Not sure whether that was intended.

You may nevertheless want a second opinion.

Thanks for the patch!

Harald


Tobias

PS: Some patch reviews are that fast that it is impossible to send the OK;
at least, I did not manage to do for Harald's last two - for the last one
I was at least 4min too late. ;-)

-
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

[Patch] Fortran: Avoid var initialization in interfaces [PR54753]

[Tobias Burnus]

Found when looking at F2018:C839 / PR54753.

For INTENT(OUT) the dummy variable (might) also be default initialized
or deallocated. However, with assumed rank, that causes issues, which
C839 prevents. In the current GCC implementation, missing C839 constraint
diagnostic, but also rejects-valid/ice-on-valid appears.

There are three issues, this patch solves the first:

* reject-valid issue due to adding the initializer also to a dummy
  argument which is in an INTERFACE block. Having initializers in
  INTERFACE blocks is pointless and causes for the attached testcase
  the bogus error:
  "Assumed-rank variable y at (1) may only be used as actual argument"

(Except for wasting resources and this error, they should be ignored
in trans*.c and usually do not cause any further harm.)


I think Sandra has a nearly ready patch to do the C839 constraint
diagnostic, which needs the attached patch to do the checks.

The third issue is that GCC currently gives either an ICE or the
above error message when declaring a procedure with a valid
assumed-rank intent(out) dummy. This has still to be solved as well.
But first I wanted to unblock Sandra's C839 work with this patch :-)


Regarding the patch, '!= IFSRC_IFBODY' has to be used; "== IFSRC_DECL"
won't work as the the generatedy ENTRY master function has IFSRC_UNKNOWN.

OK for mainline?

Tobias

PS: Some patch reviews are that fast that it is impossible to send the OK;
at least, I did not manage to do for Harald's last two - for the last one
I was at least 4min too late. ;-)

-
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
Fortran: Avoid var initialization in interfaces [PR54753]

Intent(out) implies deallocation/default initialization; however, it is
pointless to do this for dummy-arguments symbols of procedures which are
inside an INTERFACE block. – This also fixes a bogus error for the attached
included testcase, but fixing the non-interface version still has to be done.

	PR fortran/54753

gcc/fortran/ChangeLog:

	* resolve.c (can_generate_init, resolve_fl_variable_derived,
	resolve_symbol): Only do initialization with intent(out) if not
	inside of an interface block.

gcc/testsuite/ChangeLog:

	* gfortran.dg/assumed_rank_23.f90: New test.

 gcc/fortran/resolve.c | 11 ---
 gcc/testsuite/gfortran.dg/assumed_rank_23.f90 | 16 
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 30b96b2f597..5d2478d9b96 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12676,7 +12676,8 @@ can_generate_init (gfc_symbol *sym)
 || a->cray_pointer
 || sym->assoc
 || (!a->referenced && !a->result)
-|| (a->dummy && a->intent != INTENT_OUT)
+|| (a->dummy && (a->intent != INTENT_OUT
+		 || sym->ns->proc_name->attr.if_source != IFSRC_DECL))
 || (a->function && sym != sym->result)
   );
 }
@@ -12913,7 +12914,9 @@ resolve_fl_variable_derived (gfc_symbol *sym, int no_init_flag)
 
   /* Assign default initializer.  */
   if (!(sym->value || sym->attr.pointer || sym->attr.allocatable)
-  && (!no_init_flag || sym->attr.intent == INTENT_OUT))
+  && (!no_init_flag
+	  || (sym->attr.intent == INTENT_OUT
+	  && sym->ns->proc_name->attr.if_source == IFSRC_DECL)))
 sym->value = gfc_generate_initializer (>ts, can_generate_init (sym));
 
   return true;
@@ -16154,7 +16157,8 @@ resolve_symbol (gfc_symbol *sym)
 		|| sym->ts.u.derived->attr.alloc_comp
 		|| sym->ts.u.derived->attr.pointer_comp))
 	   && !(a->function && sym != sym->result))
-	  || (a->dummy && a->intent == INTENT_OUT && !a->pointer))
+	  || (a->dummy && !a->pointer && a->intent == INTENT_OUT
+	  && sym->ns->proc_name->attr.if_source == IFSRC_DECL))
 	apply_default_init (sym);
   else if (a->function && sym->result && a->access != ACCESS_PRIVATE
 	   && (sym->ts.u.derived->attr.alloc_comp
@@ -16166,6 +16170,7 @@ resolve_symbol (gfc_symbol *sym)
 
   if (sym->ts.type == BT_CLASS && sym->ns == gfc_current_ns
   && sym->attr.dummy && sym->attr.intent == INTENT_OUT
+  && sym->ns->proc_name->attr.if_source == IFSRC_DECL
   && !CLASS_DATA (sym)->attr.class_pointer
   && !CLASS_DATA (sym)->attr.allocatable)
 apply_default_init (sym);
diff --git a/gcc/testsuite/gfortran.dg/assumed_rank_23.f90 b/gcc/testsuite/gfortran.dg/assumed_rank_23.f90
new file mode 100644
index 000..c83aa7de1a3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/assumed_rank_23.f90
@@ -0,0 +1,16 @@
+! { dg-do compile }
+!
+! PR fortran/54753
+! TS29113:C535c
+! F2018:C839
+!
+module m
+
+  interface
+subroutine s1 (x, y)
+  class(*) :: x(..)
+  class(*), intent (out) :: y(..)
+end subroutine
+  end interface
+
+end module