2016-12-02 18:13 GMT+01:00 Steve Kargl <s...@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.
I think the gfc_error_now should not be necessary with my fix, but removing the ATTRIBUTE_UNUSED is certainly a good idea. > 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. I am now regtesting the attached version and, if successful, will commit. Cheers, Janus
Index: gcc/fortran/check.c =================================================================== --- gcc/fortran/check.c (revision 243194) +++ gcc/fortran/check.c (working copy) @@ -3667,7 +3667,7 @@ gfc_check_range (gfc_expr *x) bool -gfc_check_rank (gfc_expr *a ATTRIBUTE_UNUSED) +gfc_check_rank (gfc_expr *a) { /* Any data object is allowed; a "data object" is a "constant (4.1.3), variable (6), or subobject of a constant (2.4.3.2.3)" (F2008, 1.3.45). */ 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)