This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [PATCH] PR fortran/78618 -- RANK() should not ICE
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.
--
Steve