[fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
Checks
Commit Message
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 <pault@gcc.gnu.org>
Steven G. Kargl <kargl@gcc.gnu.org>
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.
Comments
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 <pault@gcc.gnu.org>
> Steven G. Kargl <kargl@gcc.gnu.org>
>
> 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.
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.
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.
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.
@@ -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");
@@ -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 <ian_harvey@bigpond.com>
+!
+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