Re: [Patch,Fortran] Add polymorphic type support (OOP, RTTI) - merge from Fortran-dev branch

[Daniel Kraft <d at domob dot eu>]

Hi,

Tobias Burnus wrote:
> attached patch adds more OOP; it is just the difference between the 4.5
> trunk and the Fortran-dev branch (with one additional line).

here are some comments while reading through the patch:

+	  if (argss == gfc_ss_terminator)
+            {
+	      gfc_conv_expr_reference (&parmse, e);
+	      tmp = fold_convert (TREE_TYPE (data),
+				  parmse.expr);
+	      gfc_add_modify (&parmse.pre, data, tmp);
+	    }

It seems the white-space of the opening curly brace is messed up.


+  if (derived->attr.extension)
+    return gfc_get_ultimate_derived_super_type (derived);
+  else
+    return derived;

I like the tail-recursive style here instead of a loop :)

+  (*as) = NULL;  /* XXX */

What's about this one?

+/* Counter for assigning a unique vindex number to each derived type.  */
+static int vindex_counter = 0;
+
+
 /* Match the beginning of a derived type declaration.  If a type name
    was the result of a function, then it is possible to have a symbol
    already to be known as a derived type yet have no components.  */
@@ -6823,6 +6896,10 @@ gfc_match_derived_decl (void)
       st->n.sym = sym;
     }

+  if (!sym->vindex)
+    /* Set the vindex for this type and increment the counter.  */
+    sym->vindex = ++vindex_counter;
+

I'm not really sure about the way vindices are calculated and this looks 
quite fine...  But is this safe even with types in multiple modules 
combined and stuff like that?  Or different compilation units?

+  if (!pointer && !proc_pointer
+	&& !(lvalue->ts.type == BT_CLASS
+		&& lvalue->ts.u.derived->components->attr.pointer))

I'm not sure how you handle this, but maybe you could set the attributes 
on the container to the same as the declared type component and get rid 
of checks like that?  Would that work?

+  if (sym->ts.type == BT_CLASS)
+    {
+      allocatable = sym->ts.u.derived->components->attr.allocatable;
+      pointer = sym->ts.u.derived->components->attr.pointer;
+    }
+  else
+    {
+      allocatable = sym->attr.allocatable;
+      pointer = sym->attr.pointer;
+    }

Ditto (and some others like that).

+	case EXEC_SELECT_TYPE:
+	  /* Do nothing. SELECT TYPE statements should be transformed into
+	  an ordinary SELECT CASE at resolution stage.
+	  TODO: Add an error message here once this is done.  */
+	  res = NULL_TREE;
+	  break;

What's with this, shouldn't there be an unreachable or assertion, now 
that I assume the transformation is really already done?

+	  /* Kill the dead block, but not the blocks below it.  */

...which raises the important question, if something dead can be killed? 
 Just ignore me here, though :)

Otherwise, the patch looks good from my point of view; and those are 
also quite minor, I think...  I would't claim I've done a full review, 
but I think it should be ok for the merge.

Yours,
Daniel

--
Done:  Arc-Bar-Cav-Ran-Rog-Sam-Tou-Val-Wiz
To go: Hea-Kni-Mon-Pri