| Message ID | CAKwh3qjN3QB=Kmr2m6yfpiPqZvUTg7dwtQbeBubSpbpCE0TdyQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 02, 2016 at 06:00:48PM +0100, Janus Weil wrote: > 2016-12-02 17:30 GMT+01:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > > On Fri, Dec 02, 2016 at 04:47:19PM +0100, Janus Weil wrote: > >> Hi Steve, > >> > >> 2016-12-02 2:33 GMT+01:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > >> > The attached patch fixes an ICE, a nearby whitespace issue, and > >> > removed the ATTRIBUTE_UNUSED tag. THe change has passed regression > >> > testing on x86_64-*-freebsd. Ok to commit? > >> > >> huh, I don't really understand why the argument of RANK is detected to > >> be an EXPR_FUNCTION for the test case at hand. Shouldn't it rather be > >> an EXPR_CONSTANT? > >> > >> Some debugging in gfc_check_rank shows that a->symtree->n.sym indeed > >> is the symbol "c" (as expected), but that clearly is not a function, > >> so it seems to me that the actual bug here is that a->expr_type is set > >> incorrectly ...? > > > > I found that it is the function __convert_s4_s1. > > That's strange. If we see different things here, maybe we are running > into some kind of undefined behavior (possibly related to > gfc_bad_expr?). Anyway, after some more debugging I came to the > conclusion that what actually fails is the error propagation, which > seems to be broken in gfc_check_assign and can be fixed like this: > > > Index: gcc/fortran/expr.c > =================================================================== > --- gcc/fortran/expr.c (revision 243194) > +++ gcc/fortran/expr.c (working copy) > @@ -3314,9 +3314,9 @@ gfc_check_assign (gfc_expr *lvalue, gfc_expr *rval > if (lvalue->ts.type == BT_CHARACTER && rvalue->ts.type == BT_CHARACTER) > { > if (lvalue->ts.kind != rvalue->ts.kind && allow_convert) > - gfc_convert_chartype (rvalue, &lvalue->ts); > - > - return true; > + return gfc_convert_chartype (rvalue, &lvalue->ts); > + else > + return true; > } > > if (!allow_convert) > > > This also avoids the ICE and I think is the proper way to fix this ... > When developing the patch, I fixed/avoided the ICE, first. Then, I tried Gerhard's second testcase without the PARAMETER attribute. An error message is emitted, so I went hunting for why PARAMETER suppressed the error message. This led to the second part of the patch of changing gfc_error to gfc_error_now. Perhaps, forcing the gfc_error_now prevents gfortran from inserting the __convert_s4_s1 call. In any event, if your patch causes an error message to be emitted, then I think that your patch is better than the one I proposed. Feel free to commit your patch.
Index: gcc/fortran/expr.c =================================================================== --- gcc/fortran/expr.c (revision 243194) +++ gcc/fortran/expr.c (working copy) @@ -3314,9 +3314,9 @@ gfc_check_assign (gfc_expr *lvalue, gfc_expr *rval if (lvalue->ts.type == BT_CHARACTER && rvalue->ts.type == BT_CHARACTER) { if (lvalue->ts.kind != rvalue->ts.kind && allow_convert) - gfc_convert_chartype (rvalue, &lvalue->ts); - - return true; + return gfc_convert_chartype (rvalue, &lvalue->ts); + else + return true; } if (!allow_convert)