Re: [Patch, Fortran] PR 57022: [4.7/4.8/4.9 Regression] Inappropriate warning for use of TRANSFER with arrays

[Janus Weil <janus at gcc dot gnu dot org>]

Hi Mikael,


> Two comments below:
>
>> Index: gcc/fortran/check.c
>> ===================================================================
>> --- gcc/fortran/check.c       (revision 198108)
>> +++ gcc/fortran/check.c       (working copy)
>> @@ -4456,7 +4455,7 @@ gfc_calculate_transfer_sizes (gfc_expr *source, gf
>>      return false;
>>
>>    /* Calculate the size of the source.  */
>> -  if (source->expr_type == EXPR_ARRAY
>> +  if ((source->expr_type == EXPR_ARRAY || source->rank > 0)
>>
> Minor: we can probably assume that rank > 0 if expr_type == EXPR_ARRAY,
> which makes the first condition unnecessary.
> (There is another instance of this later)

yes, I was also suspecting the same (but wasn't quite sure). I have
removed it now.


>> Index: gcc/fortran/target-memory.c
>> ===================================================================
>> --- gcc/fortran/target-memory.c       (revision 198108)
>> +++ gcc/fortran/target-memory.c       (working copy)
>> +/* Return the size of an expression in its target representation.  */
>> +
>> +size_t
>> +gfc_target_expr_size (gfc_expr *e)
>> +{
>> +  mpz_t tmp;
>> +  size_t asz;
>> +
>> +  gcc_assert (e != NULL);
>> +
>> +  if ((e->expr_type == EXPR_ARRAY || e->rank > 0) && gfc_array_size (e, &tmp))
>> +    asz = mpz_get_ui (tmp);
>> +  else
>> +    asz = 1;
>> +
>> +  return asz * gfc_element_size (e);
>> +}
>>
> If gfc_array_size returns false, the function return
> gfc_element_size(e), which feels wrong (or confusing at least).
> The original gfc_target_expr_size function returned 0 if it couldn't
> determine the size (in the CHARACTER case); I think we should stick to
> that.  And then, the callers should handle that case.

Good point. I have modified it accordingly, and this also means we can
fully remove the call to 'gfc_array_size' in
'gfc_calculate_transfer_sizes' (i.e. the code you commented on above),
since this is now done by 'gfc_target_expr_size'.


> The rest looks good.

Thanks a lot for the review! I will commit the updated patch in the
attachment after another regtest if there are no further comments.

Cheers,
Janus

Attachment: pr57022_v3.diff
Description: Binary data