Bug 46328

This one is related to PR 46262: Both contain polymorphic type-bound operator calls with "non-trivial" operands. In the example here one might still just "optimize away" the parenthesis, but in more general cases like

u = (u*2.)*3.

one needs to insert a temporary (ditto for PR42626), so that this line is translated into something like:

{
  class$field tmp;

  tmp = u.$vptr->multiply_real (u,2.);

  u = tmp.$vptr->multiply_real (tmp,3.);
}

This does not work yet.

i think the code below is another example of this bug.  Any chance this will be resolved in 4.7?  (The NAG Fortran and IBM XL Fortran compilers compile the code without error):

$ cat integrand.F90
module foo_module
  type ,abstract :: foo
  contains
    procedure(t_interface) ,deferred :: t
    procedure(assign_interface) ,deferred :: assign
    procedure(multiply_interface) ,deferred :: multiply
    generic :: operator(*) => multiply
    generic :: assignment(=) => assign
  end type
  abstract interface
    function t_interface(this)
      import :: foo 
      class(foo) :: this
      class(foo), allocatable ::t_interface
    end function 
    function multiply_interface(lhs,rhs) 
      import :: foo 
      class(foo), allocatable :: multiply_interface
      class(foo), intent(in) :: lhs
      real, intent(in) :: rhs
    end function 
    subroutine assign_interface(lhs,rhs) 
      import :: foo 
      class(foo), intent(in) :: rhs
      class(foo), intent(inout) :: lhs
    end subroutine 
  end interface
contains
  subroutine bar(x,dt)    
    class(foo) :: x
    real, intent(in) :: dt     
    x = x%t()*dt
  end subroutine 
end module

$ gfortran -c integrand.F90
integrand.F90:32.8:

    x = x%t()*dt
        1
Error: Operands of binary numeric operator '*' at (1) are CLASS(foo)/REAL(4)

$ gfortran --version
GNU Fortran (GCC) 4.6.1 20110325 (prerelease)
Copyright (C) 2011 Free Software Foundation, Inc.

GNU Fortran comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of GNU Fortran
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING

Note that the segfault on the code in comment #0 is gone by now. With 4.6 and trunk one currently gets:

  u = (u)*2.
      1
Error: Operands of binary numeric operator '*' at (1) are CLASS(field)/REAL(4)

(which is the same error as on comment #2.)

This error is due to the fact that the operator is not replaced by a TBP call (which in turn is due to the parenthesis, which is not optimized away).


Also note that comment #0 is invalid (since it is missing a defined assignment for type 'field'), as 4.5 reports:

  u = (u)*2.
  1
Error: Variable must not be polymorphic in assignment at (1)

Thanks for the update.  The same error occurs when a defined assignment is added:


$ cat abstract_field_expression.F90 
module field_module
  implicit none
  type ,abstract :: field
  contains
    procedure(field_op_real) ,deferred :: multiply_real
    procedure(field_eq_field) ,deferred :: assign
    generic :: operator(*) => multiply_real
    generic :: assignment(=) => assign
  end type
  abstract interface
    function field_op_real(lhs,rhs)
      import :: field
      class(field) ,intent(in)  :: lhs
      real ,intent(in) :: rhs
      class(field) ,allocatable :: field_op_real
    end function
    subroutine field_eq_field(lhs,rhs)
      import :: field
      class(field) ,intent(inout)  :: lhs
      class(field) ,intent(in)  :: rhs
    end subroutine
  end interface
end module

program main
  use field_module
  implicit none
  class(field) ,pointer :: u
  u = (u)*2.
end program

$ gfortran abstract_field_expression.F90 
abstract_field_expression.F90:29.6:

  u = (u)*2.
      1
Error: Operands of binary numeric operator '*' at (1) are CLASS(field)/REAL(4)

$ gfortran --version
GNU Fortran (GCC) 4.6.2 20111019 (prerelease)

For the code in comment #2, there is actually a problem with the 'class_ok' attribute, which can be fixed with the following patchlet:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (revision 180696)
+++ gcc/fortran/resolve.c       (working copy)
@@ -11497,6 +11497,8 @@ resolve_fl_derived0 (gfc_symbol *sym)
              c->attr.recursive = ifc->attr.recursive;
              c->attr.always_explicit = ifc->attr.always_explicit;
              c->attr.ext_attr |= ifc->attr.ext_attr;
+             if (c->ts.type == BT_CLASS)
+               c->attr.class_ok = ifc->attr.class_ok;
              /* Replace symbols in array spec.  */
              if (c->as)
                {

This gets us a bit further in 'matching_typebound_op' and changes the error on comment #2 into an ICE.

What is further needed for comment #2, is to insert a temporary to evaluate the call to x%t(), so that we can use the result as pass object and vptr base for the 'multiply' generic. The following patchlet sketches where this could happen (only flagging an ICE for the problematic case):

Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c     (revision 180696)
+++ gcc/fortran/interface.c     (working copy)
@@ -3204,13 +3204,22 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu
                             gfc_expr* base, gfc_typebound_proc* target,
                             const char *gname)
 {
-  e->expr_type = EXPR_COMPCALL;
-  e->value.compcall.tbp = target;
-  e->value.compcall.name = gname ? gname : "$op";
-  e->value.compcall.actual = actual;
-  e->value.compcall.base_object = base;
-  e->value.compcall.ignore_pass = 1;
-  e->value.compcall.assign = 0;
+  if (base->expr_type == EXPR_VARIABLE)
+    {
+      /* Generate a simple type-bound procedure call.  */
+      e->expr_type = EXPR_COMPCALL;
+      e->value.compcall.tbp = target;
+      e->value.compcall.name = gname ? gname : "$op";
+      e->value.compcall.actual = actual;
+      e->value.compcall.base_object = base;
+      e->value.compcall.ignore_pass = 1;
+      e->value.compcall.assign = 0;
+    }
+  else if (base->expr_type == EXPR_FUNCTION)
+    {
+      /* We need a temporary in order to evaluate the expression in two steps.  */
+      gfc_error ("build_compcall_for_operator: We need a temporary at %L", &e->where);
+    }
 }


The problem is that in 'build_compcall_for_operator', we only have access to the operator expression. However, we need to replace the whole gfc_code (in this case: the assignment "x=..."), preferentially by a BLOCK construct with a local temporary.

In the case at hand, things are further complicated by the fact that we need to replace the assignment itself by a type-bound call, too. Fortunately, this replacement requires no additional temporary, since the passed object is simple ("x").

One other thing that just occurred to me is that one might consider cleaning up 'gfc_extend_expr' by removing the 'real_error' argument and returning {MATCH_YES, MATCH_NO, MATCH_ERROR} instead of {SUCCESS, FAILURE}.



Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 180696)
+++ gcc/fortran/interface.c	(working copy)
@@ -3220,12 +3220,10 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu
    with the operator.  This subroutine builds an actual argument list
    corresponding to the operands, then searches for a compatible
    interface.  If one is found, the expression node is replaced with
-   the appropriate function call.
-   real_error is an additional output argument that specifies if FAILURE
-   is because of some real error and not because no match was found.  */
+   the appropriate function call.  */
 
-gfc_try
-gfc_extend_expr (gfc_expr *e, bool *real_error)
+match
+gfc_extend_expr (gfc_expr *e)
 {
   gfc_actual_arglist *actual;
   gfc_symbol *sym;

Author: pault
Date: Mon Jan  2 12:46:08 2012
New Revision: 182796

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182796
Log:
2012-01-02  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/51529
	* trans-array.c (gfc_array_allocate): Null allocated memory of
	newly allocted class arrays.

	PR fortran/46262
	PR fortran/46328
	PR fortran/51052
	* interface.c(build_compcall_for_operator): Add a type to the
	expression.
	* trans-expr.c (conv_base_obj_fcn_val): New function.
	(gfc_conv_procedure_call): Use base_expr to detect non-variable
	base objects and, ensuring that there is a temporary variable,
	build up the typebound call using conv_base_obj_fcn_val.
	(gfc_trans_class_assign): Pick out class procedure pointer
	assignments and do the assignment with no further prcessing.
	(gfc_trans_class_array_init_assign, gfc_trans_class_init_assign
	gfc_trans_class_assign): Move to top of file.
	* gfortran.h : Add 'base_expr' field to gfc_expr.
	* resolve.c (get_declared_from_expr): Add 'types' argument to
	switch checking of derived types on or off.
	(resolve_typebound_generic_call): Set the new argument.
	(resolve_typebound_function, resolve_typebound_subroutine):
	Set 'types' argument for get_declared_from_expr appropriately.
	Identify base expression, if not a variable, in the argument
	list of class valued calls. Assign it to the 'base_expr' field
	of the final expression. Strip away all references after the
	last class reference.


2012-01-02  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/46262
	PR fortran/46328
	PR fortran/51052
	* gfortran.dg/typebound_operator_7.f03: New.
	* gfortran.dg/typebound_operator_8.f03: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/typebound_operator_7.f03
    trunk/gcc/testsuite/gfortran.dg/typebound_operator_8.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/dump-parse-tree.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog

Fixed on trunk.

Thanks for the report

Paul

Thanks for the fix!  I'm very excited about the way 4.7 is shaping up.  It appears this will be a very significant release for those interested in the more advanced capabilities of OOP.

Damian

REOPEN.

The issue is mostly fixed (i.e. polymorphic operators work), but not completely. As Dominique pointed out [1], the parentheses in
   u = (u)*2.
still confuse gfortran (it works without).


Some preliminary analysis what goes wrong is available at [2, 3]:

a) In gfc_build_class_symbol, the attr->class_ok does not propagate to
   fclass->attr.class_ok  (should it?)

b) In matching_typebound_op, checking an EXPR_OP with
   gfc_expr_attr (base->expr).class_ok fails - should on use
   base->expr->ts->u.derived->attr.class_ok ?

c) In get_declared_from_expr (called by resolve_typebound_function):
   The following is wrong (ice - segfault) for an EXPR_OP:
     if (declared == NULL)
       declared = e->symtree->n.sym->ts.u.derived;
   should one use e->ts.u.derived?

(Regarding (base->expr,e)->ts.u.derived: I vaguely recall that sometimes e->ts did not have the proper data and only e->symtree->n.sym->ts had. I don't recall the details and it might have been only needed with some draft patch. It might be that e->ts.u.derived was NULL, but it could have been also something different.)

[1] http://gcc.gnu.org/ml/fortran/2012-01/msg00045.html
[2] http://gcc.gnu.org/ml/fortran/2012-01/msg00049.html
[3] http://gcc.gnu.org/ml/fortran/2012-01/msg00050.html

The following patchlet fixes the issue of comment 2 [or at least, the program now compiles]; it does not fix the issue of comment 0 / comment 4.

Regarding the latter:

One gets now a segfault for:
  at 0x50C5A0: gfc_add_component_ref(gfc_expr*, char const*) (class.c:63)
  by 0x57BDFC: resolve_typebound_function(gfc_expr*) (resolve.c:5958)
  by 0x573D74: gfc_resolve_expr(gfc_expr*) (resolve.c:6280)

which is because of:
63        gfc_symbol *derived = e->symtree->n.sym->ts.u.derived;
which fails as we have: e->expr_type == EXPR_FUNCTION


If one adds to the example in comment 2:
  use foo_module
  class(foo), pointer :: xx
  xx = xx * 4
  end
one gets:
  Error: Operands of binary numeric operator '*' at (1)
         are CLASS(foo)/INTEGER(4)
I have not checked whether that's correct or whether it should have worked.


--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3172,3 +3172,6 @@ matching_typebound_op (gfc_expr** tb_base,
          {
-           if (!gfc_expr_attr (base->expr).class_ok)
+           if ((base->expr->expr_type == EXPR_VARIABLE
+                && !gfc_expr_attr (base->expr).class_ok)
+               || (base->expr->expr_type != EXPR_VARIABLE
+                   && !base->expr->ts.u.derived->components))
              continue;
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 79245ce..8d02d6e 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -5685,4 +5685,6 @@ get_declared_from_expr (gfc_ref **class_ref, gfc_ref **new_ref,

-  if (declared == NULL)
+  if (declared == NULL && e->expr_type == EXPR_VARIABLE)
     declared = e->symtree->n.sym->ts.u.derived;
+  else
+    declared = e->ts.u.derived;

Author: burnus
Date: Mon Jan  9 23:23:26 2012
New Revision: 183039

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183039
Log:
2012-01-09  Tobias Burnus  <burnus@net-b.de>

        PR fortran/46328
        * gfortran.dg/typebound_operator_11.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/typebound_operator_11.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Remaining issues were FIXED by the commit for PR 51791.

The (u)*2 issue of comment 0 / comment 4 had already a test case; comment 12 added the test case of comment 2 to the test suite.