Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
On Tue, May 09, 2023 at 08:35:00PM +0200, Harald Anlauf wrote: > On 5/9/23 20:29, Steve Kargl via Gcc-patches wrote: > > > > It's not needed. See above. gfc_state_stack->previous is referenced > > a few lines above the if-stmt. The reference will segfault if the > > pointer is NULL. > > > > You're absolutely right. So it is OK as is. Thanks for keeping us honest and the review. -- Steve
Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
On 5/9/23 20:29, Steve Kargl via Gcc-patches wrote:
On Tue, May 09, 2023 at 08:24:16PM +0200, Harald Anlauf wrote:
Hi Paul,
On 5/9/23 17:51, Paul Richard Thomas via Gcc-patches wrote:
Hi All,
Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because
this testcase checked that finalizable derived types could not be specified
in a submodule. I have replaced the original test with a test of the patch.
Thanks also to Malcolm Cohen for guidance on this.
OK for trunk?
the patch looks good to me. However:
@@ -11637,8 +11637,9 @@ gfc_match_final_decl (void)
block = gfc_state_stack->previous->sym;
^
See below.
gcc_assert (block);
- if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous
- || gfc_state_stack->previous->previous->state != COMP_MODULE)
+ if (gfc_state_stack->previous->previous
+ && gfc_state_stack->previous->previous->state != COMP_MODULE
+ && gfc_state_stack->previous->previous->state != COMP_SUBMODULE)
{
gfc_error ("Derived type declaration with FINAL at %C must be in
the"
" specification part of a MODULE");
I am wondering if we should keep the protection against a potential
NULL pointer dereference (i.e. gfc_state_stack->previous == NULL) for
possibly invalid code. I have failed to produce a simple testcase,
but others may have "better" ideas.
It's not needed. See above. gfc_state_stack->previous is referenced
a few lines above the if-stmt. The reference will segfault if the
pointer is NULL.
You're absolutely right. So it is OK as is.
Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
On Tue, May 09, 2023 at 08:24:16PM +0200, Harald Anlauf wrote:
> Hi Paul,
>
> On 5/9/23 17:51, Paul Richard Thomas via Gcc-patches wrote:
> > Hi All,
> >
> > Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because
> > this testcase checked that finalizable derived types could not be specified
> > in a submodule. I have replaced the original test with a test of the patch.
> >
> > Thanks also to Malcolm Cohen for guidance on this.
> >
> > OK for trunk?
>
> the patch looks good to me. However:
>
> @@ -11637,8 +11637,9 @@ gfc_match_final_decl (void)
>block = gfc_state_stack->previous->sym;
^
See below.
>gcc_assert (block);
>
> - if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous
> - || gfc_state_stack->previous->previous->state != COMP_MODULE)
> + if (gfc_state_stack->previous->previous
> + && gfc_state_stack->previous->previous->state != COMP_MODULE
> + && gfc_state_stack->previous->previous->state != COMP_SUBMODULE)
> {
>gfc_error ("Derived type declaration with FINAL at %C must be in
> the"
> " specification part of a MODULE");
>
> I am wondering if we should keep the protection against a potential
> NULL pointer dereference (i.e. gfc_state_stack->previous == NULL) for
> possibly invalid code. I have failed to produce a simple testcase,
> but others may have "better" ideas.
It's not needed. See above. gfc_state_stack->previous is referenced
a few lines above the if-stmt. The reference will segfault if the
pointer is NULL.
--
Steve
Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
Hi Paul,
On 5/9/23 17:51, Paul Richard Thomas via Gcc-patches wrote:
Hi All,
Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because
this testcase checked that finalizable derived types could not be specified
in a submodule. I have replaced the original test with a test of the patch.
Thanks also to Malcolm Cohen for guidance on this.
OK for trunk?
the patch looks good to me. However:
@@ -11637,8 +11637,9 @@ gfc_match_final_decl (void)
block = gfc_state_stack->previous->sym;
gcc_assert (block);
- if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous
- || gfc_state_stack->previous->previous->state != COMP_MODULE)
+ if (gfc_state_stack->previous->previous
+ && gfc_state_stack->previous->previous->state != COMP_MODULE
+ && gfc_state_stack->previous->previous->state != COMP_SUBMODULE)
{
gfc_error ("Derived type declaration with FINAL at %C must be in
the"
" specification part of a MODULE");
I am wondering if we should keep the protection against a potential
NULL pointer dereference (i.e. gfc_state_stack->previous == NULL) for
possibly invalid code. I have failed to produce a simple testcase,
but others may have "better" ideas.
I'll leave it to you to amend the patch or leave as is.
Thanks,
Harald
Paul
Fortran: Allow declaration of finalizable DT in a submodule [PR97122]
2023-05-09 Paul Thomas
Steven G. Kargl
gcc/fortran
PR fortran/97122
* decl.cc (variable_decl): Clean up white space issues.
(gfc_match_final_decl): Declaration of finalizable derived type
is allowed in a submodule.
gcc/testsuite/
PR fortran/97122
* gfortran.dg/finalize_8.f03 : Replace testcase that checks
declaration of finalizable derived types in submodules works.
[Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
Hi All,
Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because
this testcase checked that finalizable derived types could not be specified
in a submodule. I have replaced the original test with a test of the patch.
Thanks also to Malcolm Cohen for guidance on this.
OK for trunk?
Paul
Fortran: Allow declaration of finalizable DT in a submodule [PR97122]
2023-05-09 Paul Thomas
Steven G. Kargl
gcc/fortran
PR fortran/97122
* decl.cc (variable_decl): Clean up white space issues.
(gfc_match_final_decl): Declaration of finalizable derived type
is allowed in a submodule.
gcc/testsuite/
PR fortran/97122
* gfortran.dg/finalize_8.f03 : Replace testcase that checks
declaration of finalizable derived types in submodules works.
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 233bf244d62..6d6ce0854de 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -2698,7 +2698,7 @@ variable_decl (int elem)
}
gfc_seen_div0 = false;
-
+
/* F2018:C830 (R816) An explicit-shape-spec whose bounds are not
constant expressions shall appear only in a subprogram, derived
type definition, BLOCK construct, or interface body. */
@@ -2769,7 +2769,7 @@ variable_decl (int elem)
if (e->expr_type != EXPR_CONSTANT)
{
n = gfc_copy_expr (e);
- if (!gfc_simplify_expr (n, 1) && gfc_seen_div0)
+ if (!gfc_simplify_expr (n, 1) && gfc_seen_div0)
{
m = MATCH_ERROR;
goto cleanup;
@@ -2784,12 +2784,12 @@ variable_decl (int elem)
if (e->expr_type != EXPR_CONSTANT)
{
n = gfc_copy_expr (e);
- if (!gfc_simplify_expr (n, 1) && gfc_seen_div0)
+ if (!gfc_simplify_expr (n, 1) && gfc_seen_div0)
{
m = MATCH_ERROR;
goto cleanup;
}
-
+
if (n->expr_type == EXPR_CONSTANT)
gfc_replace_expr (e, n);
else
@@ -11637,8 +11637,9 @@ gfc_match_final_decl (void)
block = gfc_state_stack->previous->sym;
gcc_assert (block);
- if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous
- || gfc_state_stack->previous->previous->state != COMP_MODULE)
+ if (gfc_state_stack->previous->previous
+ && gfc_state_stack->previous->previous->state != COMP_MODULE
+ && gfc_state_stack->previous->previous->state != COMP_SUBMODULE)
{
gfc_error ("Derived type declaration with FINAL at %C must be in the"
" specification part of a MODULE");
diff --git a/gcc/testsuite/gfortran.dg/finalize_8.f03 b/gcc/testsuite/gfortran.dg/finalize_8.f03
index b2027a0ba6d..b7fa10dda31 100644
--- a/gcc/testsuite/gfortran.dg/finalize_8.f03
+++ b/gcc/testsuite/gfortran.dg/finalize_8.f03
@@ -1,35 +1,49 @@
-! { dg-do compile }
-
-! Parsing of finalizer procedure definitions.
-! Check that FINAL-declarations are only allowed on types defined in the
-! specification part of a module.
-
-MODULE final_type
+! { dg-do run }
+!
+! PR97122: Declaration of a finalizable derived type in a submodule
+! IS allowed.
+!
+! Contributed by Ian Harvey
+!
+MODULE m
IMPLICIT NONE
-CONTAINS
+ INTERFACE
+MODULE SUBROUTINE other(i)
+ IMPLICIT NONE
+ integer, intent(inout) :: i
+END SUBROUTINE other
+ END INTERFACE
- SUBROUTINE bar
-IMPLICIT NONE
+ integer :: mi
-TYPE :: mytype
- INTEGER, ALLOCATABLE :: fooarr(:)
- REAL :: foobar
-CONTAINS
- FINAL :: myfinal ! { dg-error "in the specification part of a MODULE" }
-END TYPE mytype
-
- CONTAINS
+END MODULE m
-SUBROUTINE myfinal (el)
- TYPE(mytype) :: el
-END SUBROUTINE myfinal
+SUBMODULE (m) s
+ IMPLICIT NONE
- END SUBROUTINE bar
+ TYPE :: t
+integer :: i
+ CONTAINS
+FINAL :: final_t ! Used to be an error here
+ END TYPE t
-END MODULE final_type
+CONTAINS
-PROGRAM finalizer
- IMPLICIT NONE
- ! Do nothing here
-END PROGRAM finalizer
+ SUBROUTINE final_t(arg)
+TYPE(t), INTENT(INOUT) :: arg
+mi = -arg%i
+ END SUBROUTINE final_t
+
+ module subroutine other(i) ! 'ti' is finalized
+integer, intent(inout) :: i
+type(t) :: ti
+ti%i = i
+ END subroutine other
+END SUBMODULE s
+
+ use m
+ integer :: i = 42
+ call other(i)
+ if (mi .ne. -i) stop 1
+end
