Re: [Patch, Fortran] Fix OPTIONAL, esp. with polymorphism

[Janus Weil <janus at gcc dot gnu dot org>]

Hi Tobias,

> gfortran supports OPTIONAL since quite some time - also some more
> complicated cases involving ELEMENTAL or the new F2008 addition, but as
> testing showed, the support is still incomplete; especially with polymorphic
> arguments there were several bugs.
>
> Besides a simple absent argument, passing an absent argument on also has to
> be supported. Fortran 2008 in addition added that a deallocated allocatable
> and an unassociated pointer also counts as absent - if (and only if) it is
> passed to a nonallocatable, nonpointer optional dummy.
>
> As complication comes on top of it: The CLASS container; especially for
> class->type, type->class, class->(parent)class and when combined with
> arrays, coarrays or assumed-rank arguments. There, one needs to ensure that
> one passes the NULL correctly and that a NULL pointers doesn't get
> dereferenced.
>
> On the way, I fixed some other issues like passing polymorphic coarray
> scalars (i.e. changing a class container with array descriptor to a class
> container without array descriptor).
>
> There are still issues with ELEMENTAL and with creating an array descriptor
> for an (absent) optional array which has no array descriptor. In addition,
> for CLASS->TYPE of assumed-rank arrays, the "packaging" (creating a
> contiguous array) support is also still lacking. See the 146 commented FIXME
> lines in the patch. However, I think the patch is large enough and
> sufficiently complete to be committed without the remaining parts.
>
> Build and regtested on x86-64-linux.
> OK for the trunk?

thanks for this patch. It looks mostly ok to me. Since Dominique has
already inspected the test cases, I only looked at the patch itself. A
few minor comments:


@@ -231,12 +231,16 @@ class_array_data_assign (stmtblock_t *block,
tree lhs_desc, tree rhs_desc,

 /* Takes a derived type expression and returns the address of a temporary
    class object of the 'declared' type.  If vptr is not NULL, this is
-   used for the temporary class object.  */
+   used for the temporary class object.
+   alloc_ptr is false when the dummy is neither allocatable
+   nor a pointer; that's only relevant for the optional handling.  */
 void
 gfc_conv_derived_to_class (gfc_se *parmse, gfc_expr *e,
-			   gfc_typespec class_ts, tree vptr)
+			   gfc_typespec class_ts, tree vptr, bool optional,
+			   bool optional_alloc_ptr)

In the comment, 'alloc_ptr' should be 'optional_alloc_ptr'.



+
+static void
+class_scalar_coarray_to_class (gfc_se *parmse, gfc_expr *e,
+			       gfc_typespec class_ts, bool optional)
+{

How about a small comment preceding this function, to shortly describe
its functionality and arguments? And then inside ...


+  for (ref = e->ref; ref; ref = ref->next)
+    {
+      if (ref->type == REF_COMPONENT
+	    && ref->u.c.component->ts.type == BT_CLASS)
+	class_ref = ref;
+
+      if (ref->next == NULL)
+	break;
+    }

... I guess the last if statement is not needed, since this condition
is already checked by the for loop.



@@ -323,19 +451,26 @@ gfc_conv_derived_to_class (gfc_se *parmse, gfc_expr *e,
    type.
    OOP-TODO: This could be improved by adding code that branched on
    the dynamic type being the same as the declared type. In this case
-   the original class expression can be passed directly.  */
+   the original class expression can be passed directly.
+   alloc_ptr is false when the dummy is neither allocatable
+   nor a pointer; that's relevant for the optional handling.  */
 void
-gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e,
-			 gfc_typespec class_ts, bool elemental)
+gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts,
+			 bool elemental, bool copyback, bool optional,
+		         bool optional_alloc_ptr)

Again: 'alloc_ptr' -> 'optional_alloc_ptr' in the comment. And how
about a short comment on the 'copyback' argument?



That's pretty much all I found. Ok for trunk with the above ...

Cheers,
Janus