[Patch, Fortran] PR 41586: Allocatable _scalars_ are never auto-deallocated

[Janus Weil]

2009/10/19 Janus Weil <janus@gcc.gnu.org>:
> 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.

Committed as r152988.

Cheers,
Janus