diff mbox

[Fortran,OOP] PR 78848: [7 Regression] ICE on writing CLASS variable with non-typebound DTIO procedure

Message ID CAKwh3qgeme8NB8W3dPSsu8N2wA-TqtAaW+z9CgcUoHm8+UjYcQ@mail.gmail.com
State New
Headers show

Commit Message

Janus Weil Dec. 18, 2016, 12:12 p.m. UTC
Hi all,

the attached patch fixes an ICE on a valid DTIO example, which is in
fact a regression of one of my recent patches. See bugzilla for
details.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2016-12-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78848
    * trans-io.c (get_dtio_proc): Generate non-typebound DTIO call for class
    variables, if no typebound DTIO procedure is available.

2016-12-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78848
    * gfortran.dg/dtio_22.f90: New test.

Comments

Paul Richard Thomas Dec. 18, 2016, 12:42 p.m. UTC | #1
Hi Janus,

Thanks for taking on the cleaning up of the dtio for me. I am up to my
eyeballs in things other than gfortran.

This patch is OK for trunk since it fixes a regression.

Looking at it, though, I realised that dynamic dispatch would be
possible. If, as each derived type is being resolved, specific DTIO
procedures are found in generic interfaces, they could be added to the
derived type's vtable. This is possibly an unnecessary embellishment
but it could be done. Please commit this fix for now, however.

Thanks

Paul

On 18 December 2016 at 13:12, Janus Weil <janus@gcc.gnu.org> wrote:
> Hi all,
>
> the attached patch fixes an ICE on a valid DTIO example, which is in
> fact a regression of one of my recent patches. See bugzilla for
> details.
>
> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus
>
>
> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/78848
>     * trans-io.c (get_dtio_proc): Generate non-typebound DTIO call for class
>     variables, if no typebound DTIO procedure is available.
>
> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/78848
>     * gfortran.dg/dtio_22.f90: New test.
Janus Weil Dec. 18, 2016, 1:31 p.m. UTC | #2
Hi Paul,

> Thanks for taking on the cleaning up of the dtio for me. I am up to my
> eyeballs in things other than gfortran.
>
> This patch is OK for trunk since it fixes a regression.

thanks for the review. Committed as r243784.


> Looking at it, though, I realised that dynamic dispatch would be
> possible. If, as each derived type is being resolved, specific DTIO
> procedures are found in generic interfaces, they could be added to the
> derived type's vtable. This is possibly an unnecessary embellishment
> but it could be done. Please commit this fix for now, however.

Yes, I agree that technically it would be possible to do this.
However, I doubt that the standard requires us to do it. I just had a
look in chapter 9.5.3.7.3 of the F03 standard, which says:

----

(2) A suitable user-defined derived-type input/output procedure is
available; that is, either
(a) the declared type of the effective item has a suitable generic
type-bound procedure,
or
(b) a suitable generic interface is accessible.

If (2a) is true, the procedure referenced is determined as for
explicit type-bound procedure references
(12.4); that is, the binding with the appropriate specific interface
is located in the declared type of the
effective item, and the corresponding binding in the dynamic type of
the effective item is selected.

If (2a) is false and (2b) is true, the reference is to the procedure
identified by the appropriate specific
interface in the interface block. This reference shall not be to a
dummy procedure that is not present,
or to a disassociated procedure pointer.

---

To me this sound like a typebound DTIO procedure should indeed behave
differently than a non-typebound one, thus I think what you're
proposing is not necessary. Do you agree?

Cheers,
Janus




> On 18 December 2016 at 13:12, Janus Weil <janus@gcc.gnu.org> wrote:
>> Hi all,
>>
>> the attached patch fixes an ICE on a valid DTIO example, which is in
>> fact a regression of one of my recent patches. See bugzilla for
>> details.
>>
>> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/78848
>>     * trans-io.c (get_dtio_proc): Generate non-typebound DTIO call for class
>>     variables, if no typebound DTIO procedure is available.
>>
>> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/78848
>>     * gfortran.dg/dtio_22.f90: New test.
>
>
>
> --
> If you're walking down the right path and you're willing to keep
> walking, eventually you'll make progress.
>
> Barack Obama
Paul Richard Thomas Dec. 18, 2016, 6:16 p.m. UTC | #3
Dear Janus,

> If (2a) is false and (2b) is true, the reference is to the procedure
> identified by the appropriate specific
> interface in the interface block. This reference shall not be to a
> dummy procedure that is not present,
> or to a disassociated procedure pointer.
>

I also reread this part of the standard and thought that it was rather
ambiguous. What determines "appropriate specific interface"? That's
why I asked the question.

>
> To me this sound like a typebound DTIO procedure should indeed behave
> differently than a non-typebound one, thus I think what you're
> proposing is not necessary. Do you agree?

I think that in the circumstances, it is not.

Ciao

Paul


>
> Cheers,
> Janus
>
>
>
>
>> On 18 December 2016 at 13:12, Janus Weil <janus@gcc.gnu.org> wrote:
>>> Hi all,
>>>
>>> the attached patch fixes an ICE on a valid DTIO example, which is in
>>> fact a regression of one of my recent patches. See bugzilla for
>>> details.
>>>
>>> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/78848
>>>     * trans-io.c (get_dtio_proc): Generate non-typebound DTIO call for class
>>>     variables, if no typebound DTIO procedure is available.
>>>
>>> 2016-12-18  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/78848
>>>     * gfortran.dg/dtio_22.f90: New test.
>>
>>
>>
>> --
>> If you're walking down the right path and you're willing to keep
>> walking, eventually you'll make progress.
>>
>> Barack Obama
diff mbox

Patch

Index: gcc/fortran/trans-io.c
===================================================================
--- gcc/fortran/trans-io.c	(revision 243776)
+++ gcc/fortran/trans-io.c	(working copy)
@@ -2180,9 +2180,31 @@  get_dtio_proc (gfc_typespec * ts, gfc_code * code,
       formatted = true;
     }
 
-  if (ts->type == BT_DERIVED)
+  if (ts->type == BT_CLASS)
+    derived = ts->u.derived->components->ts.u.derived;
+  else
+    derived = ts->u.derived;
+
+  gfc_symtree *tb_io_st = gfc_find_typebound_dtio_proc (derived,
+						  last_dt == WRITE, formatted);
+  if (ts->type == BT_CLASS && tb_io_st)
     {
-      derived = ts->u.derived;
+      // polymorphic DTIO call  (based on the dynamic type)
+      gfc_se se;
+      gfc_expr *expr = gfc_find_and_cut_at_last_class_ref (code->expr1);
+      gfc_add_vptr_component (expr);
+      gfc_add_component_ref (expr,
+			     tb_io_st->n.tb->u.generic->specific_st->name);
+      *dtio_sub = tb_io_st->n.tb->u.generic->specific->u.specific->n.sym;
+      gfc_init_se (&se, NULL);
+      se.want_pointer = 1;
+      gfc_conv_expr (&se, expr);
+      gfc_free_expr (expr);
+      return se.expr;
+    }
+  else
+    {
+      // non-polymorphic DTIO call (based on the declared type)
       *dtio_sub = gfc_find_specific_dtio_proc (derived, last_dt == WRITE,
 					      formatted);
 
@@ -2189,32 +2211,8 @@  get_dtio_proc (gfc_typespec * ts, gfc_code * code,
       if (*dtio_sub)
 	return gfc_build_addr_expr (NULL, gfc_get_symbol_decl (*dtio_sub));
     }
-  else if (ts->type == BT_CLASS)
-    {
-      gfc_symtree *tb_io_st;
 
-      derived = ts->u.derived->components->ts.u.derived;
-      tb_io_st = gfc_find_typebound_dtio_proc (derived,
-					       last_dt == WRITE, formatted);
-      if (tb_io_st)
-	{
-	  gfc_se se;
-	  gfc_expr *expr = gfc_find_and_cut_at_last_class_ref (code->expr1);
-	  gfc_add_vptr_component (expr);
-	  gfc_add_component_ref (expr,
-				 tb_io_st->n.tb->u.generic->specific_st->name);
-	  *dtio_sub = tb_io_st->n.tb->u.generic->specific->u.specific->n.sym;
-	  gfc_init_se (&se, NULL);
-	  se.want_pointer = 1;
-	  gfc_conv_expr (&se, expr);
-	  gfc_free_expr (expr);
-	  return se.expr;
-	}
-    }
-
-
   return NULL_TREE;
-
 }
 
 /* Generate the call for a scalar transfer node.  */