This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [Patch, Fortran] PR 41586: Allocatable _scalars_ are never auto-deallocated
Hi Tobias,
> - ?if (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp)
> + ?/* Remember this variable for allocation/cleanup. ?*/
> + ?if (sym->attr.dimension || sym->attr.allocatable
> + ? ? ?|| (sym->ts.type == BT_CLASS &&
> + ? ? ? ? (sym->ts.u.derived->components->attr.dimension
> + ? ? ? ? ?|| sym->ts.u.derived->components->attr.allocatable)))
> ? ? gfc_defer_symbol_init (sym);
> + ?else if (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp)
> + ? ?gfc_defer_symbol_init (sym);
>
>
> I would prefer if you could add the condition of "else if" as "||" to
> the first block:
>
> + ?if (sym->attr.dimension || sym->attr.allocatable
> + ? ? ?|| (sym->ts.type == BT_CLASS &&
> + ? ? ? ? (sym->ts.u.derived->components->attr.dimension
> + ? ? ? ? ?|| sym->ts.u.derived->components->attr.allocatable))
> + ? ? ?|| (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp))
> ? ? gfc_defer_symbol_init (sym);
>
> I think that's more readable - otherwise one thinks that for BT_DERIVED
> a different action will be done - and as another "||" does not require
> more indention, I think it also does not decrease the readability.
I agree with your preference. The reason why I did not do this was
that there is another 'else if' branch below, which also does the same
thing (i.e. call gfc_defer_symbol_init), and I just added one more
branch.
If one does this, then it should be:
--- gcc/fortran/trans-decl.c (Revision 152974)
+++ gcc/fortran/trans-decl.c (Arbeitskopie)
@@ -1187,23 +1187,24 @@ gfc_get_symbol_decl (gfc_symbol * sym)
/* Create variables to hold the non-constant bits of array info. */
gfc_build_qualified_array (decl, sym);
- /* Remember this variable for allocation/cleanup. */
- gfc_defer_symbol_init (sym);
-
if ((sym->attr.allocatable || !sym->attr.dummy) && !sym->attr.pointer)
GFC_DECL_PACKED_ARRAY (decl) = 1;
}
- if (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp)
+ /* Remember this variable for allocation/cleanup. */
+ if (sym->attr.dimension || sym->attr.allocatable
+ || (sym->ts.type == BT_CLASS &&
+ (sym->ts.u.derived->components->attr.dimension
+ || sym->ts.u.derived->components->attr.allocatable))
+ || (sym->ts.type == BT_DERIVED && sym->ts.u.derived->attr.alloc_comp)
+ /* This applies a derived type default initializer. */
+ || (sym->ts.type == BT_DERIVED
+ && sym->attr.save == SAVE_NONE
+ && !sym->attr.data
+ && !sym->attr.allocatable
+ && (sym->value && !sym->ns->proc_name->attr.is_main_program)
+ && !sym->attr.use_assoc))
gfc_defer_symbol_init (sym);
- /* This applies a derived type default initializer. */
- else if (sym->ts.type == BT_DERIVED
- && sym->attr.save == SAVE_NONE
- && !sym->attr.data
- && !sym->attr.allocatable
- && (sym->value && !sym->ns->proc_name->attr.is_main_program)
- && !sym->attr.use_assoc)
- gfc_defer_symbol_init (sym);
gfc_finish_var_decl (decl, sym);
Will commit shortly (with this change). Thanks for the review.
Cheers,
Janus