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

[Janus Weil <janus at gcc dot gnu dot 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.

Cheers,
Janus