Re: [PATCH] fortran: [PR53800] Wrong copy-in/out with CLASS array to assumed-shape TYPE

On 8/5/26 3:25 AM, Mikael Morin wrote:
> Hello,
> 
> Le 30/07/2026 à 19:21, Jerry D a écrit :
>> See the attached patch.
>>
>> As stated in the PR we were missing the stride multiplier (ie span)
>>
>> I will note as a useful tool: I had an initial patch for this but wanted to 
>> exercise it. I prompted Claude Pro Opus 5 to create numerous variations on the 
>> original test case provided by Tobias Burnus in the original report. The 
>> result was 25 to 30 variations which were helpful in identifying code paths I 
>> would not have otherwise found.  I am not using those test cases here.

 From your comments below, obviously not all the test cases I needed were 
generated. lol

>>
>> I flagged out this one to work on as it has been around a long time and we 
>> were giving wrong results without any idea it was happening.
>>
>> I also wanted to mention that the fortran standardese is difficult to 
>> interpret but I think I got this right.
>>
>> Regression tested on x86_64.
>>
>> OK for mainline?
>>
>> Comments appreciated.
>>
>> Regards,
>>
>> Jerry
>> ---
>>      fortran: [PR53800] Wrong copy-in/out with CLASS array to assumed- shape TYPE
>>
>>      A CLASS array actual passed to a plain assumed-shape derived-type dummy
>>      was always copied in and out, so pointers associated with the dummy went
>>      stale on return.  F2023 15.5.2.5 paragraph 9 requires the dummy to be
>>      associated with the actual's storage when it has the TARGET attribute and
>>      lacks CONTIGUOUS and VALUE; paragraph 8 leaves every other case free to
>>      use a copy.  Pass such a dummy by reference and address its elements with
>>      the descriptor's runtime span.
>>
> 
>> diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
>> index 7b28ecfce47..4063bbe70c4 100644
>> --- a/gcc/fortran/trans.h
>> +++ b/gcc/fortran/trans.h
>> @@ -1070,6 +1070,7 @@ struct GTY(()) lang_decl {
>>    unsigned int scalar_pointer : 1;
>>    unsigned int scalar_target : 1;
>>    unsigned int optional_arg : 1;
>> +  unsigned int span_array : 1;
>>  };
>>
>>
>> @@ -1087,12 +1088,16 @@ struct GTY(()) lang_decl {
>>    (DECL_LANG_SPECIFIC (node)->scalar_target)
>>  #define GFC_DECL_OPTIONAL_ARGUMENT(node) \
>>    (DECL_LANG_SPECIFIC (node)->optional_arg)
>> +#define GFC_DECL_SPAN_ARRAY_P(node) \
>> +  (DECL_LANG_SPECIFIC (node)->span_array)
>>  #define GFC_DECL_GET_SCALAR_ALLOCATABLE(node) \
>>    (DECL_LANG_SPECIFIC (node) ? GFC_DECL_SCALAR_ALLOCATABLE (node) : 0)
>>  #define GFC_DECL_GET_SCALAR_POINTER(node) \
>>    (DECL_LANG_SPECIFIC (node) ? GFC_DECL_SCALAR_POINTER (node) : 0)
>>  #define GFC_DECL_GET_SCALAR_TARGET(node) \
>>    (DECL_LANG_SPECIFIC (node) ? GFC_DECL_SCALAR_TARGET (node) : 0)
>> +#define GFC_DECL_GET_SPAN_ARRAY_P(node) \
>> +  (DECL_LANG_SPECIFIC (node) ? GFC_DECL_SPAN_ARRAY_P (node) : 0)
>>  #define GFC_DECL_PACKED_ARRAY(node) DECL_LANG_FLAG_0(node)
>>  #define GFC_DECL_PARTIAL_PACKED_ARRAY(node) DECL_LANG_FLAG_1(node)
>>  #define GFC_DECL_ASSIGN(node) DECL_LANG_FLAG_2(node)
> 
> I have the impression that the information provided by the new flag is somehow 
> redundant with the existing flag GFC_DECL_PTR_ARRAY_P, and indeed, need_span is 
> used everywhere side by side with is_pointer_array.
> Could the flags be merged together, or maybe just reuse GFC_DECL_PTR_ARRAY_P?
>

I need to investigate this further. I was following the pattern of the defines 
just above.

--- snip ---
> 
> The check on type is probably unneeded.
> Here is a (failing) variation of the testcase with integer type:
> 

This is a good catch, your examples that fail are a key to this. I am working 
through a followup that includes these new cases, dropping the type check.

> diff --git a/gcc/testsuite/gfortran.dg/class_to_type_5.f90 b/gcc/testsuite/ 
> gfortran.dg/class_to_type_5.f90

--- snip --->
> For the rest, the two get_array_span would need to be reworked to have the flags 
> merged, but I think that's doable, and the rest looks good.

Agree, I will see how this ripples through.

Thanks for this review. Very helpful

> 
> Mikael
>