User account creation filtered due to spam.

Bug 46897 - [OOP] type-bound defined ASSIGNMENT(=) not used for derived type component in intrinsic assign
Summary: [OOP] type-bound defined ASSIGNMENT(=) not used for derived type component in...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: ---
Assignee: Paul Thomas
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2010-12-11 22:19 UTC by Tobias Burnus
Modified: 2013-09-15 14:19 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-12-13 09:28:27


Attachments
An almost fix for the PR (1.49 KB, patch)
2012-08-11 13:18 UTC, Paul Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2010-12-11 22:19:57 UTC
Reported by Damian at http://gcc.gnu.org/ml/fortran/2010-12/msg00063.html

The following program should print "defined assignment called". It does so with the IBM compiler but not with gfortran.

module component_parent_child_module
  implicit none
  type component
  contains
    procedure :: assign
    generic :: assignment(=)=>assign
  end type
  type parent
    type(component) :: foo
  end type
  type, extends(parent) :: child
  end type
contains
  subroutine assign(lhs,rhs)
    class(component), intent(out) :: lhs
    class(component), intent(in) :: rhs
    print *,'defined assignment called'
  end subroutine 
  type(child) function new_child()
  end function
end module 

program main
  use component_parent_child_module
  implicit none
  type(child) :: infant
  infant = new_child()
end
Comment 1 janus 2010-12-12 22:15:30 UTC
Here is a slightly reduced test case:


module m
  implicit none

  type component
  contains
    procedure :: assign
    generic :: assignment(=)=>assign
  end type

  type t
    type(component) :: foo
  end type

contains

  subroutine assign(lhs,rhs)
    class(component), intent(out) :: lhs
    class(component), intent(in) :: rhs
    print *,'defined assignment called'
  end subroutine

end module 

program main
  use m
  implicit none
  type(t) :: infant, newchild
  infant = newchild
end
Comment 2 janus 2010-12-12 22:25:52 UTC
I think one can run into the same problem already without polymorphism:


module m
  implicit none

  type component
  end type

  interface assignment(=)
    module procedure assign
  end interface

  type t
    type(component) :: foo
  end type

contains

  subroutine assign(lhs,rhs)
    type(component), intent(out) :: lhs
    type(component), intent(in) :: rhs
    print *,'defined assignment called'
  end subroutine

end module 

program main
  use m
  implicit none
  type(t) :: infant, newchild
  infant = newchild
end
Comment 3 Tobias Burnus 2010-12-13 07:37:47 UTC
Actually, I am less and less sure that this is a wrong-code issue. For

  type(t) :: infant, newchild
  infant = newchild

one has the type "T" but the defined assignment is for "component". If one compiles the program in comment 2, none of my compilers uses the defined assignment.

(When reading the program first, I somehow read that the defined assignment it for "t" and not for "component". I will later re-read the F2008 standard.)
Comment 4 janus 2010-12-13 09:07:43 UTC
(In reply to comment #3)
> Actually, I am less and less sure that this is a wrong-code issue.

I'm also a bit skeptical that this really is a bug. One should check the standard.


> For
> 
>   type(t) :: infant, newchild
>   infant = newchild
> 
> one has the type "T" but the defined assignment is for "component". If one
> compiles the program in comment 2, none of my compilers uses the defined
> assignment.

Not even IBM?

If one doesn't use the defined assignment in comment #2, I don't see why one should do so in comment #0.
Comment 5 janus 2010-12-13 09:28:27 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Actually, I am less and less sure that this is a wrong-code issue.
> 
> I'm also a bit skeptical that this really is a bug. One should check the
> standard.

Ok, I think the relevant part is the following quote from chapter 7.2.1.3 of the F08 standard (line 13):

"An intrinsic assignment where the variable is of derived type is performed as if each component of the variable were assigned from the corresponding component of expr using pointer assignment (7.2.2) for each pointer component, defined assignment for each nonpointer nonallocatable component of a type that has a type-bound defined assignment consistent with the component, intrinsic assignment for each other nonpointer nonallocatable component, ..."

Note that this mentions only *type-bound* defined assignment, which means in fact that the defined assignment should only be used in comment #0 and #1, but not in comment #2 (since it is not type-bound there).
Comment 6 Damian Rouson 2010-12-13 15:16:42 UTC
Janus, 

Thanks for finding the relevant language in the standard.  

In case it helps, what I submitted originally was a reduced example of code written by Jim Xia, who is member of both the IBM compiler test team and the Fortran standards committee.  IBM XL Fortran 13.1 calls the defined assignment in my original example (and interestingly in Janus's reduced example, although it appears from what you wrote that it is not required to do so in your reduced example).  

Unfortunately, Jim is on vacation, so I probably won't be able to find out soon what part of the standard he was relying upon when he wrote the relevant code, but I know he gave it a a lot of thought becasue that code plays a central role in an algorithm that he developed with us for the open-source ForTrilinos package (http://trilinos.sandia.gov/download/trilinos-10.6.html).  Every derived type in ForTrilinos extends a derived type analogous to the "parent" type in my original posting.  The defined assignment is needed for a reference counting scheme to prevent memory management problems.  If the compiler does not call the defined assignment, it would have significant downstream implications for all ForTrilinos derived types and for all user-defined types that aggregate or extend ForTrilinos derived types.

Damian
Comment 7 Tobias Burnus 2010-12-13 16:36:57 UTC
Richard Maine agrees with the assessment of comment 5:

- For INTERFACE-defined defined-ASSIGNMENT(=), the intrinsic assignment
  should be used (cf. example in comment 2) - correctly handled by gfortran

- For type-bound-defined defined-assignment, the defined assignment should be
  used (comment 0 and comment 1). gfortran currently mishandles this
  F2003 feature.

Cf. http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/d6220bd4aa90f368
Comment 8 Paul Thomas 2012-08-11 13:18:01 UTC
Created attachment 27990 [details]
An almost fix for the PR

This takes the last wrinkles out of the version that Alessandro and I last worked on.

The final step is to add the code for pointer components and to prepare the testcases.

Cheers

Paul
Comment 9 Paul Thomas 2012-12-01 08:00:32 UTC
Author: pault
Date: Sat Dec  1 08:00:22 2012
New Revision: 194016

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194016
Log:
2012-12-01   Alessandro Fanfarillo <alessandro.fanfarillo@gmail.com>
             Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/46897
	* gfortran.h : Add bit field 'defined_assign_comp' to
	symbol_attribute structure.
	Add primitive for gfc_add_full_array_ref.
	* expr.c (gfc_add_full_array_ref): New function.
	(gfc_lval_expr_from_sym): Call new function.
	* resolve.c (add_comp_ref): New function.
	(build_assignment): New function.
	(get_temp_from_expr): New function
	(add_code_to_chain): New function
	(generate_component_assignments): New function that calls all
	the above new functions.
	(resolve_code): Call generate_component_assignments.
	(check_defined_assignments): New function.
	(resolve_fl_derived0): Call check_defined_assignments.
	(gfc_resolve): Reset component_assignment_level in case it is
	left in a bad state by errors.


	* resolve.c (is_sym_host_assoc, resolve_procedure_interface,
	resolve_contained_fntype, resolve_procedure_expression,
	resolve_elemental_actual, resolve_global_procedure,
	is_scalar_expr_ptr, gfc_iso_c_func_interface, resolve_function,
	set_name_and_label, gfc_iso_c_sub_interface,
	resolve_specific_s0, resolve_operator, compare_bound_mpz_t,
	gfc_resolve_character_operator, resolve_typebound_function,
	gfc_resolve_expr, forall_index, remove_last_array_ref,
	conformable_arrays, resolve_allocate_expr,
	resolve_allocate_deallocate, resolve_select_type,
	resolve_transfer, resolve_where,
	gfc_resolve_where_code_in_forall, gfc_resolve_forall_body,
	gfc_count_forall_iterators, resolve_values,
	resolve_bind_c_comms, resolve_bind_c_derived_types,
	gfc_verify_binding_labels, apply_default_init,
	build_default_init_expr, apply_default_init_local,
	resolve_fl_var_and_proc, resolve_fl_procedure,
	gfc_resolve_finalizers, check_generic_tbp_ambiguity,
	resolve_typebound_intrinsic_op, resolve_typebound_procedure,
	resolve_typebound_procedures, ensure_not_abstract,
	resolve_fl_derived0, resolve_fl_parameter, resolve_symbol,
	resolve_equivalence_derived): Remove trailing white space.
	* gfortran.h : Remove trailing white space.

2012-12-01   Alessandro Fanfarillo <alessandro.fanfarillo@gmail.com>
             Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/46897
	* gfortran.dg/defined_assignment_1.f90: New test.
	* gfortran.dg/defined_assignment_2.f90: New test.
	* gfortran.dg/defined_assignment_3.f90: New test.
	* gfortran.dg/defined_assignment_4.f90: New test.
	* gfortran.dg/defined_assignment_5.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/defined_assignment_1.f90
    trunk/gcc/testsuite/gfortran.dg/defined_assignment_2.f90
    trunk/gcc/testsuite/gfortran.dg/defined_assignment_3.f90
    trunk/gcc/testsuite/gfortran.dg/defined_assignment_4.f90
    trunk/gcc/testsuite/gfortran.dg/defined_assignment_5.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/expr.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 10 Tobias Burnus 2012-12-03 10:02:05 UTC
The patch of comment 9 fixes most of the issues. However, some are still left. Cf. http://gcc.gnu.org/ml/fortran/2012-11/msg00072.html
Comment 11 Tobias Burnus 2013-05-22 09:19:22 UTC
Draft patch:

--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9299,4 +9299,5 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns)
     tmp->n.sym->attr.dimension = 0;
 
+  gfc_commit_symbol (tmp->n.sym);
   gfc_set_sym_referenced (tmp->n.sym);
   gfc_add_flavor (&tmp->n.sym->attr, FL_VARIABLE, name, NULL);
Comment 12 Tobias Burnus 2013-05-22 09:22:19 UTC
(In reply to Tobias Burnus from comment #11)
> Draft patch:

That patch should have gone to PR57364. However, it is related as it fixes a regression caused by a commit for this PR, namely be the one in comment 9.
Comment 13 Tobias Burnus 2013-09-15 14:19:11 UTC
(In reply to Tobias Burnus from comment #10)
> The patch of comment 9 fixes most of the issues. However, some are still
> left. Cf. http://gcc.gnu.org/ml/fortran/2012-11/msg00072.html

See also PR57696. The not-supported part is currently rejected with:

  if (depth > 1)
    {
      gfc_warning ("TODO: type-bound defined assignment(s) at %L not "
                   "done because multiple part array references would "
                   "occur in intermediate expressions.", &(*code)->loc);
      return;
    }