public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
@ 2012-10-11 21:50 Janus Weil
  2012-10-16 20:56 ` Janus Weil
  2012-11-25 19:45 ` Mikael Morin
  0 siblings, 2 replies; 6+ messages in thread
From: Janus Weil @ 2012-10-11 21:50 UTC (permalink / raw)
  To: gfortran, gcc-patches, Paul Thomas

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Hi all,

here is an OOP patch for the above PR, which has two disconnected parts:

1) It fixes a problem with ASSOCIATED, when it is fed a CLASS-valued
function as argument (i.e. the ICE in the bug title). This is the
trans-intrinsic part of the patch. Instead of adding the _data
component to the expr first and translating then, we now translate
first and then add the _data component.

2) It fixes an error with SELECT TYPE (which is a 4.8 regression), by
respecting the POINTER argument of the selector when building the
temporaries for the select type branches. This is the match.c part of
the patch, which looks much more complicated than it is, because I
merged two functions into one, which do essentially the same. I think
they were at some point split up by Paul, but I see no advantage this,
to be honest.

Regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Cheers,
Janus


2012-10-11  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/54881
	* match.c (select_derived_set_tmp,select_class_set_tmp): Removed and
	unified into ...
	(select_type_set_tmp): ... this one. Set POINTER argument according to
	selector.
	* trans-intrinsic.c (gfc_conv_associated): Use 'gfc_class_data_get'
	instead of 'gfc_add_data_component'.

2012-10-11  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/54881
	* gfortran.dg/associated_6.f90: New.
	* gfortran.dg/select_type_30.f03: New.

[-- Attachment #2: pr54881.diff --]
[-- Type: application/octet-stream, Size: 5883 bytes --]

Index: gcc/fortran/trans-intrinsic.c
===================================================================
--- gcc/fortran/trans-intrinsic.c	(revision 192159)
+++ gcc/fortran/trans-intrinsic.c	(working copy)
@@ -5732,8 +5732,6 @@ gfc_conv_associated (gfc_se *se, gfc_expr *expr)
   gfc_init_se (&arg1se, NULL);
   gfc_init_se (&arg2se, NULL);
   arg1 = expr->value.function.actual;
-  if (arg1->expr->ts.type == BT_CLASS)
-    gfc_add_data_component (arg1->expr);
   arg2 = arg1->next;
 
   /* Check whether the expression is a scalar or not; we cannot use
@@ -5755,7 +5753,10 @@ gfc_conv_associated (gfc_se *se, gfc_expr *expr)
 	      && arg1->expr->symtree->n.sym->attr.dummy)
 	    arg1se.expr = build_fold_indirect_ref_loc (input_location,
 						       arg1se.expr);
-	  tmp2 = arg1se.expr;
+	  if (arg1->expr->ts.type == BT_CLASS)
+	      tmp2 = gfc_class_data_get (arg1se.expr);
+	  else
+	    tmp2 = arg1se.expr;
         }
       else
         {
@@ -5790,6 +5791,8 @@ gfc_conv_associated (gfc_se *se, gfc_expr *expr)
 	      && arg1->expr->symtree->n.sym->attr.dummy)
 	    arg1se.expr = build_fold_indirect_ref_loc (input_location,
 						       arg1se.expr);
+	  if (arg1->expr->ts.type == BT_CLASS)
+	    arg1se.expr = gfc_class_data_get (arg1se.expr);
 
 	  arg2se.want_pointer = 1;
 	  gfc_conv_expr (&arg2se, arg2->expr);
Index: gcc/fortran/match.c
===================================================================
--- gcc/fortran/match.c	(revision 192159)
+++ gcc/fortran/match.c	(working copy)
@@ -5207,104 +5207,57 @@ select_type_push (gfc_symbol *sel)
 }
 
 
-/* Set the temporary for the current derived type SELECT TYPE selector.  */
+/* Set up a temporary for the current TYPE IS / CLASS IS branch .  */
 
-static gfc_symtree *
-select_derived_set_tmp (gfc_typespec *ts)
+static void
+select_type_set_tmp (gfc_typespec *ts)
 {
   char name[GFC_MAX_SYMBOL_LEN];
   gfc_symtree *tmp;
-  
-  sprintf (name, "__tmp_type_%s", ts->u.derived->name);
-  gfc_get_sym_tree (name, gfc_current_ns, &tmp, false);
-  gfc_add_type (tmp->n.sym, ts, NULL);
 
-  /* Copy across the array spec to the selector.  */
-  if (select_type_stack->selector->ts.type == BT_CLASS
-      && select_type_stack->selector->attr.class_ok
-      && (CLASS_DATA (select_type_stack->selector)->attr.dimension
-	  || CLASS_DATA (select_type_stack->selector)->attr.codimension))
+  if (!ts)
     {
-      tmp->n.sym->attr.dimension
-		= CLASS_DATA (select_type_stack->selector)->attr.dimension;
-      tmp->n.sym->attr.codimension
-		= CLASS_DATA (select_type_stack->selector)->attr.codimension;
-      tmp->n.sym->as
-	= gfc_copy_array_spec (CLASS_DATA (select_type_stack->selector)->as);
+      select_type_stack->tmp = NULL;
+      return;
     }
-
-  gfc_set_sym_referenced (tmp->n.sym);
-  gfc_add_flavor (&tmp->n.sym->attr, FL_VARIABLE, name, NULL);
-  tmp->n.sym->attr.select_type_temporary = 1;
-
-  return tmp;
-}
-
-
-/* Set the temporary for the current class SELECT TYPE selector.  */
-
-static gfc_symtree *
-select_class_set_tmp (gfc_typespec *ts)
-{
-  char name[GFC_MAX_SYMBOL_LEN];
-  gfc_symtree *tmp;
   
-  if (select_type_stack->selector->ts.type == BT_CLASS
-      && !select_type_stack->selector->attr.class_ok)
-    return NULL;
+  if (!gfc_type_is_extensible (ts->u.derived))
+    return;
 
-  sprintf (name, "__tmp_class_%s", ts->u.derived->name);
+  if (ts->type == BT_CLASS)
+    sprintf (name, "__tmp_class_%s", ts->u.derived->name);
+  else
+    sprintf (name, "__tmp_type_%s", ts->u.derived->name);
   gfc_get_sym_tree (name, gfc_current_ns, &tmp, false);
   gfc_add_type (tmp->n.sym, ts, NULL);
 
-/* Copy across the array spec to the selector.  */
   if (select_type_stack->selector->ts.type == BT_CLASS
-      && (CLASS_DATA (select_type_stack->selector)->attr.dimension
+      && select_type_stack->selector->attr.class_ok)
+    {
+      tmp->n.sym->attr.pointer
+		= CLASS_DATA (select_type_stack->selector)->attr.class_pointer;
+
+      /* Copy across the array spec to the selector.  */
+      if ((CLASS_DATA (select_type_stack->selector)->attr.dimension
 	  || CLASS_DATA (select_type_stack->selector)->attr.codimension))
-    {
-      tmp->n.sym->attr.pointer = 1;
-      tmp->n.sym->attr.dimension
-		= CLASS_DATA (select_type_stack->selector)->attr.dimension;
-      tmp->n.sym->attr.codimension
-		= CLASS_DATA (select_type_stack->selector)->attr.codimension;
-      tmp->n.sym->as
-	= gfc_copy_array_spec (CLASS_DATA (select_type_stack->selector)->as);
+	{
+	  tmp->n.sym->attr.dimension
+		    = CLASS_DATA (select_type_stack->selector)->attr.dimension;
+	  tmp->n.sym->attr.codimension
+		    = CLASS_DATA (select_type_stack->selector)->attr.codimension;
+	  tmp->n.sym->as
+	    = gfc_copy_array_spec (CLASS_DATA (select_type_stack->selector)->as);
+	}
     }
 
   gfc_set_sym_referenced (tmp->n.sym);
   gfc_add_flavor (&tmp->n.sym->attr, FL_VARIABLE, name, NULL);
   tmp->n.sym->attr.select_type_temporary = 1;
-  gfc_build_class_symbol (&tmp->n.sym->ts, &tmp->n.sym->attr,
-			  &tmp->n.sym->as, false);
 
-  return tmp;
-}
-
-
-static void
-select_type_set_tmp (gfc_typespec *ts)
-{
-  gfc_symtree *tmp;
-
-  if (!ts)
-    {
-      select_type_stack->tmp = NULL;
-      return;
-    }
-  
-  if (!gfc_type_is_extensible (ts->u.derived))
-    return;
-
-  /* Logic is a LOT clearer with separate functions for class and derived
-     type temporaries! There are not many more lines of code either.  */
   if (ts->type == BT_CLASS)
-    tmp = select_class_set_tmp (ts);
-  else
-    tmp = select_derived_set_tmp (ts);
+    gfc_build_class_symbol (&tmp->n.sym->ts, &tmp->n.sym->attr,
+			    &tmp->n.sym->as, false);
 
-  if (tmp == NULL)
-    return;
-
   /* Add an association for it, so the rest of the parser knows it is
      an associate-name.  The target will be set during resolution.  */
   tmp->n.sym->assoc = gfc_get_association_list ();

[-- Attachment #3: associated_6.f90 --]
[-- Type: application/octet-stream, Size: 610 bytes --]

! { dg-do run }
!
! PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
!
! Contributed by Richard L Lozes <richard@lozestech.com>

  implicit none

  type treeNode
    type(treeNode), pointer :: right => null()
  end type

  type(treeNode) :: n

  if (associated(RightOf(n))) call abort()
  allocate(n%right)
  if (.not.associated(RightOf(n))) call abort()
  deallocate(n%right)
  
contains

  function RightOf (theNode)
    class(treeNode), pointer :: RightOf
    type(treeNode), intent(in) :: theNode
    RightOf => theNode%right
  end function
  
end

[-- Attachment #4: select_type_30.f03 --]
[-- Type: application/octet-stream, Size: 620 bytes --]

! { dg-do compile }
!
! PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
!
! Contributed by Richard L Lozes <richard@lozestech.com>

  implicit none

  type treeNode
  end type

  class(treeNode), pointer :: theNode
  logical :: lstatus
  
  select type( theNode )
  type is (treeNode)
    call DestroyNode (theNode, lstatus )
  class is (treeNode)
    call DestroyNode (theNode, lstatus )
  end select
  
contains

  subroutine DestroyNode( theNode, lstatus )
    type(treeNode), pointer :: theNode
    logical, intent(out) :: lstatus
  end subroutine
  
end 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
  2012-10-11 21:50 [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016 Janus Weil
@ 2012-10-16 20:56 ` Janus Weil
  2012-10-29 20:43   ` Janus Weil
  2012-11-25 19:45 ` Mikael Morin
  1 sibling, 1 reply; 6+ messages in thread
From: Janus Weil @ 2012-10-16 20:56 UTC (permalink / raw)
  To: gfortran, gcc-patches, Paul Thomas

ping!


2012/10/11 Janus Weil <janus@gcc.gnu.org>:
> Hi all,
>
> here is an OOP patch for the above PR, which has two disconnected parts:
>
> 1) It fixes a problem with ASSOCIATED, when it is fed a CLASS-valued
> function as argument (i.e. the ICE in the bug title). This is the
> trans-intrinsic part of the patch. Instead of adding the _data
> component to the expr first and translating then, we now translate
> first and then add the _data component.
>
> 2) It fixes an error with SELECT TYPE (which is a 4.8 regression), by
> respecting the POINTER argument of the selector when building the
> temporaries for the select type branches. This is the match.c part of
> the patch, which looks much more complicated than it is, because I
> merged two functions into one, which do essentially the same. I think
> they were at some point split up by Paul, but I see no advantage this,
> to be honest.
>
> Regtested on x86_64-unknown-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus
>
>
> 2012-10-11  Janus Weil  <janus@gcc.gnu.org>
>
>         PR fortran/54881
>         * match.c (select_derived_set_tmp,select_class_set_tmp): Removed and
>         unified into ...
>         (select_type_set_tmp): ... this one. Set POINTER argument according to
>         selector.
>         * trans-intrinsic.c (gfc_conv_associated): Use 'gfc_class_data_get'
>         instead of 'gfc_add_data_component'.
>
> 2012-10-11  Janus Weil  <janus@gcc.gnu.org>
>
>         PR fortran/54881
>         * gfortran.dg/associated_6.f90: New.
>         * gfortran.dg/select_type_30.f03: New.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
  2012-10-16 20:56 ` Janus Weil
@ 2012-10-29 20:43   ` Janus Weil
  0 siblings, 0 replies; 6+ messages in thread
From: Janus Weil @ 2012-10-29 20:43 UTC (permalink / raw)
  To: gfortran, gcc-patches, Paul Thomas

ping**2


2012/10/16 Janus Weil <janus@gcc.gnu.org>:
> ping!
>
>
> 2012/10/11 Janus Weil <janus@gcc.gnu.org>:
>> Hi all,
>>
>> here is an OOP patch for the above PR, which has two disconnected parts:
>>
>> 1) It fixes a problem with ASSOCIATED, when it is fed a CLASS-valued
>> function as argument (i.e. the ICE in the bug title). This is the
>> trans-intrinsic part of the patch. Instead of adding the _data
>> component to the expr first and translating then, we now translate
>> first and then add the _data component.
>>
>> 2) It fixes an error with SELECT TYPE (which is a 4.8 regression), by
>> respecting the POINTER argument of the selector when building the
>> temporaries for the select type branches. This is the match.c part of
>> the patch, which looks much more complicated than it is, because I
>> merged two functions into one, which do essentially the same. I think
>> they were at some point split up by Paul, but I see no advantage this,
>> to be honest.
>>
>> Regtested on x86_64-unknown-linux-gnu. Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>> 2012-10-11  Janus Weil  <janus@gcc.gnu.org>
>>
>>         PR fortran/54881
>>         * match.c (select_derived_set_tmp,select_class_set_tmp): Removed and
>>         unified into ...
>>         (select_type_set_tmp): ... this one. Set POINTER argument according to
>>         selector.
>>         * trans-intrinsic.c (gfc_conv_associated): Use 'gfc_class_data_get'
>>         instead of 'gfc_add_data_component'.
>>
>> 2012-10-11  Janus Weil  <janus@gcc.gnu.org>
>>
>>         PR fortran/54881
>>         * gfortran.dg/associated_6.f90: New.
>>         * gfortran.dg/select_type_30.f03: New.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
  2012-10-11 21:50 [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016 Janus Weil
  2012-10-16 20:56 ` Janus Weil
@ 2012-11-25 19:45 ` Mikael Morin
  2012-11-26 10:35   ` Janus Weil
  1 sibling, 1 reply; 6+ messages in thread
From: Mikael Morin @ 2012-11-25 19:45 UTC (permalink / raw)
  To: Janus Weil; +Cc: gfortran, gcc-patches, Paul Thomas

Le 11/10/2012 23:49, Janus Weil a écrit :
> Hi all,
>
> here is an OOP patch for the above PR, which has two disconnected parts:
>
> 1) It fixes a problem with ASSOCIATED, when it is fed a CLASS-valued
> function as argument (i.e. the ICE in the bug title). This is the
> trans-intrinsic part of the patch. Instead of adding the _data
> component to the expr first and translating then, we now translate
> first and then add the _data component.
>
> 2) It fixes an error with SELECT TYPE (which is a 4.8 regression), by
> respecting the POINTER argument of the selector when building the
> temporaries for the select type branches. This is the match.c part of
> the patch, which looks much more complicated than it is, because I
> merged two functions into one, which do essentially the same. I think
> they were at some point split up by Paul, but I see no advantage this,
> to be honest.
>
> Regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Looks good to me. Thanks, and sorry for the delay.

Mikael

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
  2012-11-25 19:45 ` Mikael Morin
@ 2012-11-26 10:35   ` Janus Weil
  0 siblings, 0 replies; 6+ messages in thread
From: Janus Weil @ 2012-11-26 10:35 UTC (permalink / raw)
  To: Mikael Morin; +Cc: gfortran, gcc-patches, Paul Thomas

2012/11/25 Mikael Morin <mikael.morin@sfr.fr>:
> Le 11/10/2012 23:49, Janus Weil a écrit :
>
>> Hi all,
>>
>> here is an OOP patch for the above PR, which has two disconnected parts:
>>
>> 1) It fixes a problem with ASSOCIATED, when it is fed a CLASS-valued
>> function as argument (i.e. the ICE in the bug title). This is the
>> trans-intrinsic part of the patch. Instead of adding the _data
>> component to the expr first and translating then, we now translate
>> first and then add the _data component.
>>
>> 2) It fixes an error with SELECT TYPE (which is a 4.8 regression), by
>> respecting the POINTER argument of the selector when building the
>> temporaries for the select type branches. This is the match.c part of
>> the patch, which looks much more complicated than it is, because I
>> merged two functions into one, which do essentially the same. I think
>> they were at some point split up by Paul, but I see no advantage this,
>> to be honest.
>>
>> Regtested on x86_64-unknown-linux-gnu. Ok for trunk?
>
>
> Looks good to me. Thanks, and sorry for the delay.

Thanks, Mikael. Committed as r193809.

Cheers,
Janus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016
@ 2012-10-16 21:18 Dominique Dhumieres
  0 siblings, 0 replies; 6+ messages in thread
From: Dominique Dhumieres @ 2012-10-16 21:18 UTC (permalink / raw)
  To: fortran; +Cc: paul.richard.thomas, gcc-patches, janus

Janus,

Your patch works as advertised without disturbing my pet bugs.
Just a nit pick: the double parentheses in

+      if ((CLASS_DATA (select_type_stack->selector)->attr.dimension
 	  || CLASS_DATA (select_type_stack->selector)->attr.codimension))

do not seem necessary.

Note for Paul: I had to adjust the patch in order to make it compatible
with the "unlimited polymorphism" patch at
http://gcc.gnu.org/ml/fortran/2012-07/msg00130.html

Dominique

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-11-26 10:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-11 21:50 [Patch, Fortran, OOP] PR 54881: [4.8 Regression] [OOP] ICE in fold_convert_loc, at fold-const.c:2016 Janus Weil
2012-10-16 20:56 ` Janus Weil
2012-10-29 20:43   ` Janus Weil
2012-11-25 19:45 ` Mikael Morin
2012-11-26 10:35   ` Janus Weil
2012-10-16 21:18 Dominique Dhumieres

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).