2016-12-02 19:06 GMT+01:00 Janus Weil <ja...@gcc.gnu.org>:
> 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.

Testing went well. Committed as r243201.

Cheers,
Janus

Reply via email to