| Message ID | CAKwh3qgGcE5HvbAqKBTgKN2bYAoHNxs4Zu8FoVqUNZ7_uyXb+w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Dear Janus, I woke up in the middle of the night realising that not only are you right about the need for dynamic dispatch but that my dtio_20.f90 must already work. Thanks for putting me right! Paul On 13 December 2016 at 00:30, Janus Weil <janus@gcc.gnu.org> wrote: > Hi Paul, hi all, > > 2016-12-12 21:04 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >> As commented several times in bugzilla, my feeling is that the >> solution for this PR would be to utilize the vtable machinery, in >> order to generate a truly polymorphic call to the DTIO procedure. > > in order to elaborate what I have in mind, I'm attaching a draft patch > which implements polymorphic DTIO in the most straightforward manner I > could come up with. I have not regtested it yet, but at least it > removes the link failure on comment 0 and 6 in the PR and most > importantly it generates the correct output for comment 18, which none > of the previous attempts have accomplished. > > I'd be grateful for any comments, in particular whether I'm on the > right track here or whether I'm misinterpreting the F03 standard in > any way ... > > (Btw, it seems that Paul's dtio_20.f90 works already on current trunk, > so it's not very well suited to test for the problem at hand.) > > Cheers, > Janus > > PS: A quick check of the dtio_* tests shows ICEs on dtio_7.f90 and > dtio_13.f90. I'll look into those tomorrow.
Dear Janus, We got there! OK for trunk. This was a demonstration of the corollary of the "bon mot" from Barack Obama at the end of the message :-) Many thanks for finding the right path. Paul On 13 December 2016 at 10:23, Janus Weil <janus@gcc.gnu.org> wrote: > 2016-12-13 10:58 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >> 2016-12-13 10:42 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >>>> please find attached a version of your patch that runs all the dtio >>>> testcases successfully. >>> >>> Great, thanks a lot. Your addition of >>> gfc_find_and_cut_at_last_class_ref is just what I was looking for >>> right now ... >>> >>> If you don't mind I'll write a ChangeLog, add the proper tests and >>> commit to trunk. Or do you prefer to take care of it yourself? >> >> Btw, to continue the brainstorming, I think I found a slightly better >> solution for the dtio_13 problem, which even removes the spurious >> error on that test case via a small fix in resolve_transfer. The >> attached patch is what I'd like to commit if you're ok with it. > > Finally, here is a complete patch, including testcase and ChangeLog > entries. Ok for trunk? > > Cheers, > Janus > > > 2016-12-13 Janus Weil <janus@gcc.gnu.org> > Paul Thomas <pault@gcc.gnu.org> > > PR fortran/78737 > * gfortran.h (gfc_find_typebound_dtio_proc): New prototype. > * interface.c (gfc_compare_interfaces): Whitespace fix. > (gfc_find_typebound_dtio_proc): New function. > (gfc_find_specific_dtio_proc): Use it. > * resolve.c (resolve_transfer): Improve error recovery. > * trans-io.c (get_dtio_proc): Implement polymorphic calls to DTIO > procedures. > > 2016-12-13 Janus Weil <janus@gcc.gnu.org> > Paul Thomas <pault@gcc.gnu.org> > > PR fortran/78737 > * gfortran.dg/dtio_13.f90: Remove spurious error. > * gfortran.dg/dtio_19.f90: New test case.
Hi Paul, > We got there! OK for trunk. thanks. Unfortunately there was a problem with my latest patch, so what I now committed as r243609 is basically your fixed version of my draft patch (with some very minor adjustments). Phew, we finally nailed it! (My dtio_13 fix had seemed to work at some point, but in the end apparently it didn't, so I discarded it.) > This was a demonstration of the corollary of the "bon mot" from Barack > Obama at the end of the message :-) :) Maybe we can even "make gfortran great again" (to paraphrase another specimen of that category) ;P > Many thanks for finding the right path. Thanks for your help on the way, and of course thanks to Damian for triggering this whole discussion in the first place. Cheers, Janus > On 13 December 2016 at 10:23, Janus Weil <janus@gcc.gnu.org> wrote: >> 2016-12-13 10:58 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >>> 2016-12-13 10:42 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >>>>> please find attached a version of your patch that runs all the dtio >>>>> testcases successfully. >>>> >>>> Great, thanks a lot. Your addition of >>>> gfc_find_and_cut_at_last_class_ref is just what I was looking for >>>> right now ... >>>> >>>> If you don't mind I'll write a ChangeLog, add the proper tests and >>>> commit to trunk. Or do you prefer to take care of it yourself? >>> >>> Btw, to continue the brainstorming, I think I found a slightly better >>> solution for the dtio_13 problem, which even removes the spurious >>> error on that test case via a small fix in resolve_transfer. The >>> attached patch is what I'd like to commit if you're ok with it. >> >> Finally, here is a complete patch, including testcase and ChangeLog >> entries. Ok for trunk? >> >> Cheers, >> Janus >> >> >> 2016-12-13 Janus Weil <janus@gcc.gnu.org> >> Paul Thomas <pault@gcc.gnu.org> >> >> PR fortran/78737 >> * gfortran.h (gfc_find_typebound_dtio_proc): New prototype. >> * interface.c (gfc_compare_interfaces): Whitespace fix. >> (gfc_find_typebound_dtio_proc): New function. >> (gfc_find_specific_dtio_proc): Use it. >> * resolve.c (resolve_transfer): Improve error recovery. >> * trans-io.c (get_dtio_proc): Implement polymorphic calls to DTIO >> procedures. >> >> 2016-12-13 Janus Weil <janus@gcc.gnu.org> >> Paul Thomas <pault@gcc.gnu.org> >> >> PR fortran/78737 >> * gfortran.dg/dtio_13.f90: Remove spurious error. >> * gfortran.dg/dtio_19.f90: New test case. > > > > -- > If you're walking down the right path and you're willing to keep > walking, eventually you'll make progress. > > Barack Obama
Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 243580) +++ gcc/fortran/gfortran.h (working copy) @@ -3252,6 +3252,7 @@ int gfc_has_vector_subscript (gfc_expr*); gfc_intrinsic_op gfc_equivalent_op (gfc_intrinsic_op); bool gfc_check_typebound_override (gfc_symtree*, gfc_symtree*); void gfc_check_dtio_interfaces (gfc_symbol*); +gfc_symtree* gfc_find_typebound_dtio_proc (gfc_symbol *, bool, bool); gfc_symbol* gfc_find_specific_dtio_proc (gfc_symbol*, bool, bool); Index: gcc/fortran/interface.c =================================================================== --- gcc/fortran/interface.c (revision 243580) +++ gcc/fortran/interface.c (working copy) @@ -4826,13 +4826,10 @@ gfc_check_dtio_interfaces (gfc_symbol *derived) } -gfc_symbol * -gfc_find_specific_dtio_proc (gfc_symbol *derived, bool write, bool formatted) +gfc_symtree* +gfc_find_typebound_dtio_proc (gfc_symbol *derived, bool write, bool formatted) { gfc_symtree *tb_io_st = NULL; - gfc_symbol *dtio_sub = NULL; - gfc_symbol *extended; - gfc_typebound_proc *tb_io_proc, *specific_proc; bool t = false; if (!derived || derived->attr.flavor != FL_DERIVED) @@ -4869,7 +4866,20 @@ gfc_check_dtio_interfaces (gfc_symbol *derived) true, &derived->declared_at); } + return tb_io_st; +} + +gfc_symbol * +gfc_find_specific_dtio_proc (gfc_symbol *derived, bool write, bool formatted) +{ + gfc_symtree *tb_io_st = NULL; + gfc_symbol *dtio_sub = NULL; + gfc_symbol *extended; + gfc_typebound_proc *tb_io_proc, *specific_proc; + + tb_io_st = gfc_find_typebound_dtio_proc (derived, write, formatted); + if (tb_io_st != NULL) { const char *genname; Index: gcc/fortran/trans-io.c =================================================================== --- gcc/fortran/trans-io.c (revision 243580) +++ gcc/fortran/trans-io.c (working copy) @@ -2181,16 +2181,36 @@ get_dtio_proc (gfc_typespec * ts, gfc_code * code, } if (ts->type == BT_DERIVED) - derived = ts->u.derived; - else - derived = ts->u.derived->components->ts.u.derived; + { + derived = ts->u.derived; + *dtio_sub = gfc_find_specific_dtio_proc (derived, last_dt == WRITE, + formatted); - *dtio_sub = gfc_find_specific_dtio_proc (derived, last_dt == WRITE, - formatted); + if (*dtio_sub) + return gfc_build_addr_expr (NULL, gfc_get_symbol_decl (*dtio_sub)); + } + else if (ts->type == BT_CLASS) + { + gfc_expr *expr = gfc_copy_expr(code->expr1); + gfc_add_vptr_component (expr); - if (*dtio_sub) - return gfc_build_addr_expr (NULL, gfc_get_symbol_decl (*dtio_sub)); + derived = ts->u.derived->components->ts.u.derived; + gfc_symtree *tb_io_st = gfc_find_typebound_dtio_proc (derived, + last_dt == WRITE, formatted); + if (tb_io_st) + { + gfc_se se; + gfc_init_se (&se, NULL); + se.want_pointer = 1; + 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_conv_expr (&se, expr); + return se.expr; + } + } + return NULL_TREE; }