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)

Reply via email to