[Patch, fortran] PR46328 - [OOP] type-bound operator call with non-trivial polymorphic operand

[Tobias Burnus]

Dear Paul,

Paul Richard Thomas wrote:
> Bootstrapped and regtested on i686/Ubuntu 11.1 - OK for trunk?
>
> 2011-12-30  Paul Thomas<pault@gcc.gnu.org>
>
> 	* trans-array.c (gfc_array_allocate): Null allocated memory of
> 	newly allocted class arrays.

PR fortran/51529

> 	PR fortran/46328
> 	* interface.c(build_compcall_for_operator): Add a type to the
> 	expression.

You might want to quote additionally PR fortran/51052 and PR fortran/51052.

> *** gcc/fortran/interface.c	(revision 182566)
> --- gcc/fortran/interface.c	(working copy)
> ***************
> *** 3256,3261 ****
> --- 3256,3269 ----

I would have liked a diff with "-p" flag which shows the function name 
in the diff (for instance "svn diff -x -p" or "svn diff -x '-p -u'").

>
> +   if (expr->ts.type == BT_CLASS&&  expr3)
> +     {
> +       tmp = build_int_cst (unsigned_char_type_node, 0);
> +       /* With class objects, it is best to play safe and null the
> + 	 memory because we cannot know if dynamic types have allocatable
> + 	 components or not.  */

I don't like the comment; how about something along the following: "For 
class objects we need to nullify the memory in case they have 
allocatable components; the reason is that _copy, which is used for 
initialization, first frees the destination."


> + gfc_trans_class_init_assign (gfc_code *code)
> + {
> +   stmtblock_t block;
> +   tree tmp;
> +   gfc_se dst,src,memsz;

Space after each comma.

> +   gfc_expr *lhs,*rhs,*sz;

Ditto.


> ***************
> *** 3301,3306 ****
> --- 3502,3514 ----
>    		{
>    		  gfc_conv_expr_reference (&parmse, e);
>
> + 		  /* Catch base objects that are not variables.  */
> + 		  if (e->ts.type == BT_CLASS
> + 			&&  e->expr_type != EXPR_VARIABLE
> + 			&&  expr&&  e == expr->base_expr)

The indentation looks wrong.


> + 	  for (args= e->value.function.actual; args; args = args->next)
> + 	    {
> + 	      if (expr == args->expr)
> + 		expr = args->expr;
> + 	    }

Space before the equal sign in "args=". If you want, you can also remove 
the curly braces as they are not required.

> + 	  args= code->expr1->value.function.actual;
> + 	  for (; args; args = args->next)
> + 	    {
> + 	      if (expr == args->expr)
> + 		expr = args->expr;
> + 	    }

Ditto.

>    get_declared_from_expr (gfc_ref **class_ref, gfc_ref **new_ref,
> ! 			gfc_expr *e)
> --- 5623,5629 ----
>    get_declared_from_expr (gfc_ref **class_ref, gfc_ref **new_ref,
> ! 			gfc_expr *e, bool types)

I think "types" deserves a comment line in the comment block before the 
function; additionally - and related - I wonder whether the name is well 
chosen. "types" reminds me of "bt" rather than of "bool". In the 
changelog, you use: "Add 'types' argument to switch checking of derived 
types on or off." Thus, "check_types" would be a possible choice.

Otherwise, the patch looks okay to me.

Happy new year to every one!

Tobias