[patch, fortran] Implement constant-folding for TRANSFER intrinsic.

[Tobias Schlüter]

Brooks Moses wrote:
>> Brooks Moses wrote:
>>> +  /* Calculate the size of the source.  */
>>> +  if (source->expr_type == EXPR_ARRAY
>>> +      && gfc_array_size (source, &tmp) == FAILURE)
>>> +    gfc_internal_error ("Failure getting length of a constant array.");
>>
>> I think we're standardizing on gcc_assert even in the frontend.
> 
> Even for checking whether a retrieval function (such as gfc_array_size) 
> returns FAILURE?  That seems convoluted, and involves assignment to an 
> extra temporary variable, since it's wrong to put that function call 
> within the assert....

Ouch, I missed that.  Yes, you're right.  The other FEs will call 
gcc_unreachable then, but I don't think that's to be preferred over 
emitting an internal error.

>>> +      if (size)
>>              ^^^^
>> Above you checked size != NULL.  I'd prefer if you omitted that in 
>> both cases.
> 
> I'm not sure I follow how this check can legitimately be omitted in 
> either case, without producing wrong results.  The SIZE argument is 
> optional, and thus can legitimately be null.  Also, before this we 
> checked that MOLD is an array _or_ size != NULL; either one of those 
> situations will cause the result to be an array, and so this check isn't 
> redundant.
> 
> Or do you merely mean that the "NULL" part should be omitted, and the 
> earlier version of it should be written as "!size" rather than as "size 
> != NULL"?  That makes sense.

The latter, sorry for the confusion.

>>> +       result_length = (size_t)mpz_get_ui (size->value.integer);
>>> +      else
>>> +       {
>>> +         result_length = source_size / result_elt_size;
>>> +         if (result_length * result_elt_size < source_size)
>>> +           result_length += 1;
>>> +       }
>>
>> result_length = (source_size - 1) / result_elt_size + 1;
>>
>> Don't know if that's harder to understand.
> 
> I find it a bit harder to understand, personally, but I'm not very 
> familiar with this sort of integer-division trick.

My suggestion may also break with zero source_sizes, so I think we 
should stay with yours.

>> What about targets with different character sets?
> 
> They are currently hypothetical; even the S390 is only currently 
> supported in an ASCII configuration as far as I can tell.
> 
> It's a valid concern beyond that, but I'm not sure what to do with it.

I thought the S390 used EBCDIC unconditionally, if that's not the case, 
I agree that it's not a real problem.

> My inclination would be to do the cosmetic and small fixes to this and 
> go ahead and commit it and then work on the alternate-character-set 
> hypotheticality and the derived-type spacing stuff in follow-up patches, 
> since AFAIK this doesn't cause anything to break that worked 
> previously.  But that's not a strong inclination, and I'll let Paul and 
> FX make the call on that.  :)

I think we should assert that we're not dealing with derived types, 
since otherwise we would leave the user with a potentially hard-to-find 
error.  Apart from that, I agree with this course of action.

- Tobi