This is the mail archive of the
mailing list for the GNU Fortran project.
Re: [EXTERNAL] Re: [Patch, F03, RFC] FINAL support in 4.8
- From: Janus Weil <janus at gcc dot gnu dot org>
- To: "Rouson, Damian" <rouson at sandia dot gov>
- Cc: Tobias Burnus <burnus at net-b dot de>, gfortran <fortran at gcc dot gnu dot org>, Paul Thomas <paul dot richard dot thomas at gmail dot com>, Alessandro Fanfarillo <fanfarillo dot gcc at gmail dot com>
- Date: Wed, 24 Oct 2012 22:59:15 +0200
- Subject: Re: [EXTERNAL] Re: [Patch, F03, RFC] FINAL support in 4.8
- References: <50884AED.firstname.lastname@example.org> <8BD04F8FCECE2F43BE5F3D8C4EDB452D4D0AFEA5@EXMB09.srn.sandia.gov>
> It would be great if someone could check whether
> the current version of the patch correctly compiles and runs the attached
> test code (the main program at the bottom will print out whether the test
> passes or fails).
unfortunately the current version of the patch ICEs on your test case:
f951: internal compiler error: Segmentation fault
One more item for the to-do list, I guess. Thanks for the test case, though!
> On a different note, is there any chance of unlimited polymorphism making
> it into 4.8?
I think this question can only be answered by Paul ...
> On 10/24/12 1:09 PM, "Tobias Burnus" <email@example.com> wrote:
>>Janus Weil wrote:
>>> To get the balling rolling again, I'm attaching a new version of the
>>> FINAL patch, based on Tobias' 2012-09-18 version from
>>> https://userpage.physik.fu-berlin.de/~tburnus/final/. At this point I
>>> have only re-diffed it to apply cleanly to the current trunk,
>>Thanks for re-diffing and taking also a bit care of this patch.
>>I am stumbled about the following part of the patch (as it was clashing
>>with my local tree). I wonder whether those are needed or have been
>>fixed differently; they are obviously unrelated to FINAL and fix an
>>implicit-pure issue, possibly it was a first attempt for PR
>>fortran/54556 and has been fixed differently. I do not recall anything
>>about those bits and would be happy if you could investigate.
>>--------------- CUT ---------------------------
>>--- gcc/fortran/expr.c (revision 192767)
>>+++ gcc/fortran/expr.c (working copy)
>>@@ -4782,13 +4782,18 @@ gfc_check_vardef_context (gfc_expr* e, bool pointe
>> /* Variable not assignable from a PURE procedure but appears in
>> variable definition context. */
>>- if (!pointer && gfc_pure (NULL) && gfc_impure_variable (sym))
>>+ if (!pointer && gfc_impure_variable (sym))
>>- if (context)
>>+ if (gfc_implicit_pure (NULL))
>>+ gfc_current_ns->proc_name->attr.implicit_pure = 0;
>>+ else if (gfc_pure (NULL))
>>+ if (context)
>> gfc_error ("Variable '%s' can not appear in a variable definition"
>> " context (%s) at %L in PURE procedure",
>> sym->name, context, &e->where);
>>- return FAILURE;
>>+ return FAILURE;
>> if (!pointer && context && gfc_implicit_pure (NULL)
>>--------------- CUT ---------------------------
>>> fixed the scan-tree-dump regressions. Only two testsuite regressions
>>> are remaining now:
>>> FAIL: gfortran.dg/class_array_14.f90 -O0 execution test
>>> FAIL: gfortran.dg/finalize_6.f90 -O (test for excess errors)
>>> The second one is not dramatic, one only needs to put some more care
>>> into throwing error messages with -std=f95 (I will do that soon). The
>>> first one, however, throws segfaults at runtime (invalid memory
>>> reference). I don't directly see what goes wrong there.
>>Looking at the failure in gdb, is problem is the call:
>>where the dummy is:
>>class(typ1), intent(out), optional :: y1
>>And in the patch that matches trans-decl.c's init_intent_out_dt:
>>+/* FIXME: For OPTIONAL, the condition is completely missing, see: PR
>>It might be sufficient to uncomment the block below the FIXME line. (The
>>PR is about fixing issues with INTENT(OUT) plus polymorphic variables
>>[now fixed for scalars, still pending for arrays] and regarding OPTIONAL
>>[many fixes, still issues related to ELEMENTAL].)
>>I had stopped worrying about getting the FINAL code right when I already
>>encountered issues with the existing non-FINAL code in the trunk. Now,
>>as OPTIONAL mostly works and INTENT(OUT) works for polymorphic
>>*scalars*, one should now have a sufficient basis to fix this issue for
>>> In any case, I would say we should get the patch into trunk ASAP, even
>>> if the implementation of FINAL is not 100% complete (on Tobias'
>>> website, quite a number of to-do's is listed, some of which may still
>>> take a significant amount of effort). AFAICS, the patch already fixes
>>> PR 46321, i.e. polymorphic deallocation (at least in a basic form),
>>> which for me is enough justification to get it in. That is quite some
>>> progress, even if not all corner-cases of finalization will make it
>>> into 4.8.
>>My main worry is the finalization of an extended-type array, where one
>>of the ancestor types has a also a finalizer. While there shouldn't be a
>>problem with scalars or if there is no finalizer in the ancestor - or
>>only in the ancestor and not in the current type, or if no new component
>>has been added when extended, in all other cases (and also in similar
>>cases with allocatables), I fear one runs into the problem with bigger
>>real-world application (contrary to simple examples). That reason is
>>that the finalization wrapper subroutine has:
>>type(t), contiguous :: x(..) ! Assumed-rank array
>>call parent_wrapper (x%parent)
>>In the latter step, one has to pack the array. Or alternatively, one
>>could change the wrapper to use
>>subroutine generated_wrapper (x, size)
>>type(t), contiguous :: x(..)
>>integer(c_intdiff_t) :: size
>>call parent_wrapper(x%parent, size)
>>That would also work with elemental FINAL subroutines. However, it then
>>fails when calling any array finalization subroutine (in the parent
>>wrapper) as it requires packing. The pro of the latter version is that
>>one does not need to pack arrays in most of the cases, but the con is
>>that packing the array when needed gets much more tricky. For the first
>>case one only needs minimal code in trans-expr to detect this special
>>case (if assumed rank and, e.g., "expr->ts != expr->symtree->n.sym->ts")
>> and it can then use the general code (to be written) for passing
>>assumed-rank to CONTIGUOUS assumed-rank, which is already required for
>>TS29113 compliance. [If we had the new array descriptor, life would be
>>(The FINAL version is a bit easier as the memory has only a stride in
>>the lowest dimension (= different size of the type), for TS29113 one can
>>have a stride in all dimensions: In the type (for CLASS->TYPE) and in
>>array dimension 1 to rank(array). Thus, one could start with a
>>FINAL-only version. The function gfc_conv_subref_array_arg might be a
>>good starting point.)
>>(From the TODO list on the webpage.) Not really show stoppers, but I
>>think one should do some optimizations like:
>>- "Don't add the vtab->_final == NULL check if the declared type is
>>known to have a non-null finalizer."
>>And the correctness fix
>>- "Variables in the main program are finalized, but shouldn't. (Are
>>other implicitly/explicitly SAVEd vars also affected?)".
>>One probably should also do some cleanup, possibly also some refactoring
>>and code sharing. Currently, one essentially the same three times
>>(deallocate arrays, deallocate scalars, intent(out) handling; I think
>>end of scope is already handled by one of the others.) And "Check that
>>scope-leaving variables which aren't allocatable are properly
>>finalized." could also be addressed.
>>I think one can defer the big block regarding allocatable components to
>>later; I have the feeling this only affects finalizable components with
>>intrinsic assignment (and code-wise but not FINAL related: coarray
>>Thus, the tasks are:
>>1. Fixing OPTIONAL/INTENT(OUT) for FINAL - i.e. no segfault if not
>>present, but finalization with INTENT(OUT)
>>2. (Fixing INTENT(OUT) for polymorphic arrays, which is strickly
>>3. Refactoring/Cleanup/Test cases
>>4. Minor speed up: No "vtab->_final == NULL" run-time check if known to
>>5. Don't finalize local variables in the MAIN program (implicit save).
>>6. Check that finalizable nonallocatable local variables are finalized
>>at the end of the scope
>>7. Support packing.
>>I think I should work on (7), but I wouldn't mind if someone else could
>>work on (1) to (6).