Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-04-15 Thread Paul Richard Thomas via Gcc-patches
Pushed to master in commit 9a0e09f3dd5339bb18cc47317f2298d9157ced29

Thanks

Paul


On Wed, 14 Apr 2021 at 14:51, Tobias Burnus  wrote:

> On 11.04.21 09:05, Paul Richard Thomas wrote:
> > Tobias noticed a major technical fault with the resubmission below: I
> > forgot to attach the patch :-(
>
> LGTM. Plus as remarked in the first review: 'trans-expr_c' typo needs to
> be fixed (ChangeLog).
>
> Tobias
>
> >
> > Please find it attached this time.
> >
> > Paul
> >
> > On Tue, 6 Apr 2021 at 18:08, Paul Richard Thomas
> > mailto:paul.richard.tho...@gmail.com>>
> > wrote:
> >
> > Hi Tobias,
> >
> > I believe that the attached fixes the problems that you found with
> > gfc_find_and_cut_at_last_class_ref.
> >
> > I will test:
> >type1%type%array_class2 → NULL is returned  (why?)
> >class1%type%array_class2 → ts = class1 but array2_class is used
> > later on (ups!)
> >class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
> >
> > The ChangeLogs remain the same, apart from the date.
> >
> > Regtests OK on FC33/x86_64.
> >
> > Paul
> >
> >
> > On Mon, 29 Mar 2021 at 14:58, Tobias Burnus
> > mailto:tob...@codesourcery.com>> wrote:
> >
> > Hi all,
> >
> > as preremark I want to note that the testcase class_assign_4.f90
> > was added for PR83118/PR96012 (fixes problems in handling
> > class objects, Dec 18, 2020)
> > and got revised for PR99124 (class defined operators, Feb 23,
> > 2021).
> > Both patches were then also applied to GCC 9 and 10.
> >
> > On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:
> > > This patch comes in two versions: submit.diff with
> > Change.Logs or
> > > submit2.diff with Change2.Logs.
> > > The first fixes the problem by changing array temporaries
> > from class
> > > expressions into class temporaries. This permits the use of
> > > gfc_get_class_from_expr to obtain the vptr for these
> > temporaries and all
> > > the good things that come with that when handling dynamic
> > types. The second
> > > part of the fix is to use the array element length from the
> > class
> > > descriptor, when reallocating on assignment. This is needed
> > because the
> > > vptr is being set too early. I will set about trying to
> > track down why this
> > > is happening and fix it after release.
> > >
> > > The second version does the same as the first but puts in
> > place a load of
> > > tidying up that is permitted by the fix to class array
> > temporaries.
> >
> > > I couldn't readily see how to prepare a testcase - ideas?
> > > Both regtest on FC33/x86_64. The first was tested by
> > Dominique (see the
> > > PR). OK for master?
> >
> > Typo – underscore-'c' should be a dot-'c' – both changelog files
> >
> > >   * trans-expr_c (gfc_trans_scalar_assign): Make use of
> > pre and
> >
> > I think the second longer version is nicer in general, but at
> > least for
> > GCC 9/GCC10 the first version is simpler and, hence, less
> > error prone.
> >
> > As you only ask about mainline, I would prefer the second one.
> >
> > However, I am not happy about gfc_find_and_cut_at_last_class_ref:
> >
> > > + of refs following. If ts is non-null the cut is at the
> > class entity
> > > + or component that is followed by an array reference, which
> > is not +
> > > an element. */ ... + + if (ts) + { + if (e->symtree + &&
> > > e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
> > > >symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref
> > = e->ref;
> > > ref; ref = ref->next) { + if (ts && ref->type ==
> > REF_COMPONENT + &&
> > > ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
> > > ref->next->type == REF_COMPONENT + && strcmp
> > > (ref->next->u.c.component->name, "_data") == 0 + &&
> > ref->next->next +
> > > && ref->next->next->type == REF_ARRAY + &&
> > ref->next->next->u.ar.type
> > > != AR_ELEMENT) + { + *ts = >u.c.component->ts; +
> > class_ref = ref;
> > > + break; + } + + if (ts && *ts == NULL) + return NULL; +
> > Namely, if there is:
> >type1%array_class2 → array_class2 is used for 'ts' and
> > later (ok)
> >type1%type%array_class2 → NULL is returned  (why?)
> >class1%type%array_class2 → ts = class1 but array2_class is
> > used later on (ups!)
> >class1%...%scalar_class2 → ts = class1 but scalar_class2 is
> > used
> > etc.
> >
> > Thus this either needs to be cleaned up (separate 'ref' loop for
> > ts != NULL) – including 

Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-04-14 Thread Tobias Burnus

On 11.04.21 09:05, Paul Richard Thomas wrote:

Tobias noticed a major technical fault with the resubmission below: I
forgot to attach the patch :-(


LGTM. Plus as remarked in the first review: 'trans-expr_c' typo needs to
be fixed (ChangeLog).

Tobias



Please find it attached this time.

Paul

On Tue, 6 Apr 2021 at 18:08, Paul Richard Thomas
mailto:paul.richard.tho...@gmail.com>>
wrote:

Hi Tobias,

I believe that the attached fixes the problems that you found with
gfc_find_and_cut_at_last_class_ref.

I will test:
   type1%type%array_class2 → NULL is returned  (why?)
   class1%type%array_class2 → ts = class1 but array2_class is used
later on (ups!)
   class1%...%scalar_class2 → ts = class1 but scalar_class2 is used

The ChangeLogs remain the same, apart from the date.

Regtests OK on FC33/x86_64.

Paul


On Mon, 29 Mar 2021 at 14:58, Tobias Burnus
mailto:tob...@codesourcery.com>> wrote:

Hi all,

as preremark I want to note that the testcase class_assign_4.f90
was added for PR83118/PR96012 (fixes problems in handling
class objects, Dec 18, 2020)
and got revised for PR99124 (class defined operators, Feb 23,
2021).
Both patches were then also applied to GCC 9 and 10.

On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:
> This patch comes in two versions: submit.diff with
Change.Logs or
> submit2.diff with Change2.Logs.
> The first fixes the problem by changing array temporaries
from class
> expressions into class temporaries. This permits the use of
> gfc_get_class_from_expr to obtain the vptr for these
temporaries and all
> the good things that come with that when handling dynamic
types. The second
> part of the fix is to use the array element length from the
class
> descriptor, when reallocating on assignment. This is needed
because the
> vptr is being set too early. I will set about trying to
track down why this
> is happening and fix it after release.
>
> The second version does the same as the first but puts in
place a load of
> tidying up that is permitted by the fix to class array
temporaries.

> I couldn't readily see how to prepare a testcase - ideas?
> Both regtest on FC33/x86_64. The first was tested by
Dominique (see the
> PR). OK for master?

Typo – underscore-'c' should be a dot-'c' – both changelog files

>   * trans-expr_c (gfc_trans_scalar_assign): Make use of
pre and

I think the second longer version is nicer in general, but at
least for
GCC 9/GCC10 the first version is simpler and, hence, less
error prone.

As you only ask about mainline, I would prefer the second one.

However, I am not happy about gfc_find_and_cut_at_last_class_ref:

> + of refs following. If ts is non-null the cut is at the
class entity
> + or component that is followed by an array reference, which
is not +
> an element. */ ... + + if (ts) + { + if (e->symtree + &&
> e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
> >symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref
= e->ref;
> ref; ref = ref->next) { + if (ts && ref->type ==
REF_COMPONENT + &&
> ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
> ref->next->type == REF_COMPONENT + && strcmp
> (ref->next->u.c.component->name, "_data") == 0 + &&
ref->next->next +
> && ref->next->next->type == REF_ARRAY + &&
ref->next->next->u.ar.type
> != AR_ELEMENT) + { + *ts = >u.c.component->ts; +
class_ref = ref;
> + break; + } + + if (ts && *ts == NULL) + return NULL; +
Namely, if there is:
   type1%array_class2 → array_class2 is used for 'ts' and
later (ok)
   type1%type%array_class2 → NULL is returned  (why?)
   class1%type%array_class2 → ts = class1 but array2_class is
used later on (ups!)
   class1%...%scalar_class2 → ts = class1 but scalar_class2 is
used
etc.

Thus this either needs to be cleaned up (separate 'ref' loop for
ts != NULL) – including the wording in the description which
tells what
happens if 'ts' is passed as arg but the expr has rank == 0 – and
what value is assigned to 'ts'. (You can then also fix
'class.c::' to
'class.c: ' in the description above the function.)

Alternatively, you can leave the current code ref handling
code in place
at build_class_array_ref, which might be the simpler alternative.

Otherwise, it looks sensible to me.

Tobias

-
Mentor Graphics (Deutschland) GmbH, 

Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-04-11 Thread Paul Richard Thomas via Gcc-patches
Tobias noticed a major technical fault with the resubmission below: I
forgot to attach the patch :-(

Please find it attached this time.

Paul

On Tue, 6 Apr 2021 at 18:08, Paul Richard Thomas <
paul.richard.tho...@gmail.com> wrote:

> Hi Tobias,
>
> I believe that the attached fixes the problems that you found with
> gfc_find_and_cut_at_last_class_ref.
>
> I will test:
>type1%type%array_class2 → NULL is returned  (why?)
>class1%type%array_class2 → ts = class1 but array2_class is used later
> on (ups!)
>class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
>
> The ChangeLogs remain the same, apart from the date.
>
> Regtests OK on FC33/x86_64.
>
> Paul
>
>
> On Mon, 29 Mar 2021 at 14:58, Tobias Burnus 
> wrote:
>
>> Hi all,
>>
>> as preremark I want to note that the testcase class_assign_4.f90
>> was added for PR83118/PR96012 (fixes problems in handling class objects,
>> Dec 18, 2020)
>> and got revised for PR99124 (class defined operators, Feb 23, 2021).
>> Both patches were then also applied to GCC 9 and 10.
>>
>> On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:
>> > This patch comes in two versions: submit.diff with Change.Logs or
>> > submit2.diff with Change2.Logs.
>> > The first fixes the problem by changing array temporaries from class
>> > expressions into class temporaries. This permits the use of
>> > gfc_get_class_from_expr to obtain the vptr for these temporaries and all
>> > the good things that come with that when handling dynamic types. The
>> second
>> > part of the fix is to use the array element length from the class
>> > descriptor, when reallocating on assignment. This is needed because the
>> > vptr is being set too early. I will set about trying to track down why
>> this
>> > is happening and fix it after release.
>> >
>> > The second version does the same as the first but puts in place a load
>> of
>> > tidying up that is permitted by the fix to class array temporaries.
>>
>> > I couldn't readily see how to prepare a testcase - ideas?
>> > Both regtest on FC33/x86_64. The first was tested by Dominique (see the
>> > PR). OK for master?
>>
>> Typo – underscore-'c' should be a dot-'c' – both changelog files
>>
>> >   * trans-expr_c (gfc_trans_scalar_assign): Make use of pre and
>>
>> I think the second longer version is nicer in general, but at least for
>> GCC 9/GCC10 the first version is simpler and, hence, less error prone.
>>
>> As you only ask about mainline, I would prefer the second one.
>>
>> However, I am not happy about gfc_find_and_cut_at_last_class_ref:
>>
>> > + of refs following. If ts is non-null the cut is at the class entity
>> > + or component that is followed by an array reference, which is not +
>> > an element. */ ... + + if (ts) + { + if (e->symtree + &&
>> > e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
>> > >symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref = e->ref;
>> > ref; ref = ref->next) { + if (ts && ref->type == REF_COMPONENT + &&
>> > ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
>> > ref->next->type == REF_COMPONENT + && strcmp
>> > (ref->next->u.c.component->name, "_data") == 0 + && ref->next->next +
>> > && ref->next->next->type == REF_ARRAY + && ref->next->next->u.ar.type
>> > != AR_ELEMENT) + { + *ts = >u.c.component->ts; + class_ref = ref;
>> > + break; + } + + if (ts && *ts == NULL) + return NULL; +
>> Namely, if there is:
>>type1%array_class2 → array_class2 is used for 'ts' and later (ok)
>>type1%type%array_class2 → NULL is returned  (why?)
>>class1%type%array_class2 → ts = class1 but array2_class is used later
>> on (ups!)
>>class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
>> etc.
>>
>> Thus this either needs to be cleaned up (separate 'ref' loop for
>> ts != NULL) – including the wording in the description which tells what
>> happens if 'ts' is passed as arg but the expr has rank == 0 – and
>> what value is assigned to 'ts'. (You can then also fix 'class.c::' to
>> 'class.c: ' in the description above the function.)
>>
>> Alternatively, you can leave the current code ref handling code in place
>> at build_class_array_ref, which might be the simpler alternative.
>>
>> Otherwise, it looks sensible to me.
>>
>> Tobias
>>
>> -
>> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
>> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
>> Thürauf
>>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough" -
> Albert Einstein
>


-- 
"If you can't explain it simply, you don't understand it well enough" -
Albert Einstein
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index be5eb89350f..ca90142530c 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -1403,9 +1403,6 @@ gfc_trans_create_temp_array (stmtblock_t * pre, stmtblock_t * post, gfc_ss * ss,
   desc = gfc_create_var (type, "atmp");
   GFC_DECL_PACKED_ARRAY (desc) = 1;
 
-  

Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-04-10 Thread Tobias Burnus

Dear Paul,

sorry for the belate reply. I think you forgot to attach the patch.

Tobias

On 06.04.21 19:08, Paul Richard Thomas via Fortran wrote:

Hi Tobias,

I believe that the attached fixes the problems that you found with
gfc_find_and_cut_at_last_class_ref.

I will test:
type1%type%array_class2 → NULL is returned  (why?)
class1%type%array_class2 → ts = class1 but array2_class is used later on
(ups!)
class1%...%scalar_class2 → ts = class1 but scalar_class2 is used

The ChangeLogs remain the same, apart from the date.

Regtests OK on FC33/x86_64.

Paul


On Mon, 29 Mar 2021 at 14:58, Tobias Burnus  wrote:


Hi all,

as preremark I want to note that the testcase class_assign_4.f90
was added for PR83118/PR96012 (fixes problems in handling class objects,
Dec 18, 2020)
and got revised for PR99124 (class defined operators, Feb 23, 2021).
Both patches were then also applied to GCC 9 and 10.

On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:

This patch comes in two versions: submit.diff with Change.Logs or
submit2.diff with Change2.Logs.
The first fixes the problem by changing array temporaries from class
expressions into class temporaries. This permits the use of
gfc_get_class_from_expr to obtain the vptr for these temporaries and all
the good things that come with that when handling dynamic types. The

second

part of the fix is to use the array element length from the class
descriptor, when reallocating on assignment. This is needed because the
vptr is being set too early. I will set about trying to track down why

this

is happening and fix it after release.

The second version does the same as the first but puts in place a load of
tidying up that is permitted by the fix to class array temporaries.
I couldn't readily see how to prepare a testcase - ideas?
Both regtest on FC33/x86_64. The first was tested by Dominique (see the
PR). OK for master?

Typo – underscore-'c' should be a dot-'c' – both changelog files


   * trans-expr_c (gfc_trans_scalar_assign): Make use of pre and

I think the second longer version is nicer in general, but at least for
GCC 9/GCC10 the first version is simpler and, hence, less error prone.

As you only ask about mainline, I would prefer the second one.

However, I am not happy about gfc_find_and_cut_at_last_class_ref:


+ of refs following. If ts is non-null the cut is at the class entity
+ or component that is followed by an array reference, which is not +
an element. */ ... + + if (ts) + { + if (e->symtree + &&
e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
>symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref = e->ref;
ref; ref = ref->next) { + if (ts && ref->type == REF_COMPONENT + &&
ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
ref->next->type == REF_COMPONENT + && strcmp
(ref->next->u.c.component->name, "_data") == 0 + && ref->next->next +
&& ref->next->next->type == REF_ARRAY + && ref->next->next->u.ar.type
!= AR_ELEMENT) + { + *ts = >u.c.component->ts; + class_ref = ref;
+ break; + } + + if (ts && *ts == NULL) + return NULL; +

Namely, if there is:
type1%array_class2 → array_class2 is used for 'ts' and later (ok)
type1%type%array_class2 → NULL is returned  (why?)
class1%type%array_class2 → ts = class1 but array2_class is used later
on (ups!)
class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
etc.

Thus this either needs to be cleaned up (separate 'ref' loop for
ts != NULL) – including the wording in the description which tells what
happens if 'ts' is passed as arg but the expr has rank == 0 – and
what value is assigned to 'ts'. (You can then also fix 'class.c::' to
'class.c: ' in the description above the function.)

Alternatively, you can leave the current code ref handling code in place
at build_class_array_ref, which might be the simpler alternative.

Otherwise, it looks sensible to me.

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf





Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-04-06 Thread Paul Richard Thomas via Gcc-patches
Hi Tobias,

I believe that the attached fixes the problems that you found with
gfc_find_and_cut_at_last_class_ref.

I will test:
   type1%type%array_class2 → NULL is returned  (why?)
   class1%type%array_class2 → ts = class1 but array2_class is used later on
(ups!)
   class1%...%scalar_class2 → ts = class1 but scalar_class2 is used

The ChangeLogs remain the same, apart from the date.

Regtests OK on FC33/x86_64.

Paul


On Mon, 29 Mar 2021 at 14:58, Tobias Burnus  wrote:

> Hi all,
>
> as preremark I want to note that the testcase class_assign_4.f90
> was added for PR83118/PR96012 (fixes problems in handling class objects,
> Dec 18, 2020)
> and got revised for PR99124 (class defined operators, Feb 23, 2021).
> Both patches were then also applied to GCC 9 and 10.
>
> On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:
> > This patch comes in two versions: submit.diff with Change.Logs or
> > submit2.diff with Change2.Logs.
> > The first fixes the problem by changing array temporaries from class
> > expressions into class temporaries. This permits the use of
> > gfc_get_class_from_expr to obtain the vptr for these temporaries and all
> > the good things that come with that when handling dynamic types. The
> second
> > part of the fix is to use the array element length from the class
> > descriptor, when reallocating on assignment. This is needed because the
> > vptr is being set too early. I will set about trying to track down why
> this
> > is happening and fix it after release.
> >
> > The second version does the same as the first but puts in place a load of
> > tidying up that is permitted by the fix to class array temporaries.
>
> > I couldn't readily see how to prepare a testcase - ideas?
> > Both regtest on FC33/x86_64. The first was tested by Dominique (see the
> > PR). OK for master?
>
> Typo – underscore-'c' should be a dot-'c' – both changelog files
>
> >   * trans-expr_c (gfc_trans_scalar_assign): Make use of pre and
>
> I think the second longer version is nicer in general, but at least for
> GCC 9/GCC10 the first version is simpler and, hence, less error prone.
>
> As you only ask about mainline, I would prefer the second one.
>
> However, I am not happy about gfc_find_and_cut_at_last_class_ref:
>
> > + of refs following. If ts is non-null the cut is at the class entity
> > + or component that is followed by an array reference, which is not +
> > an element. */ ... + + if (ts) + { + if (e->symtree + &&
> > e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
> > >symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref = e->ref;
> > ref; ref = ref->next) { + if (ts && ref->type == REF_COMPONENT + &&
> > ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
> > ref->next->type == REF_COMPONENT + && strcmp
> > (ref->next->u.c.component->name, "_data") == 0 + && ref->next->next +
> > && ref->next->next->type == REF_ARRAY + && ref->next->next->u.ar.type
> > != AR_ELEMENT) + { + *ts = >u.c.component->ts; + class_ref = ref;
> > + break; + } + + if (ts && *ts == NULL) + return NULL; +
> Namely, if there is:
>type1%array_class2 → array_class2 is used for 'ts' and later (ok)
>type1%type%array_class2 → NULL is returned  (why?)
>class1%type%array_class2 → ts = class1 but array2_class is used later
> on (ups!)
>class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
> etc.
>
> Thus this either needs to be cleaned up (separate 'ref' loop for
> ts != NULL) – including the wording in the description which tells what
> happens if 'ts' is passed as arg but the expr has rank == 0 – and
> what value is assigned to 'ts'. (You can then also fix 'class.c::' to
> 'class.c: ' in the description above the function.)
>
> Alternatively, you can leave the current code ref handling code in place
> at build_class_array_ref, which might be the simpler alternative.
>
> Otherwise, it looks sensible to me.
>
> Tobias
>
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
> Thürauf
>


-- 
"If you can't explain it simply, you don't understand it well enough" -
Albert Einstein


Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-03-29 Thread Tobias Burnus

Hi all,

as preremark I want to note that the testcase class_assign_4.f90
was added for PR83118/PR96012 (fixes problems in handling class objects, Dec 
18, 2020)
and got revised for PR99124 (class defined operators, Feb 23, 2021).
Both patches were then also applied to GCC 9 and 10.

On 26.03.21 17:30, Paul Richard Thomas via Gcc-patches wrote:

This patch comes in two versions: submit.diff with Change.Logs or
submit2.diff with Change2.Logs.
The first fixes the problem by changing array temporaries from class
expressions into class temporaries. This permits the use of
gfc_get_class_from_expr to obtain the vptr for these temporaries and all
the good things that come with that when handling dynamic types. The second
part of the fix is to use the array element length from the class
descriptor, when reallocating on assignment. This is needed because the
vptr is being set too early. I will set about trying to track down why this
is happening and fix it after release.

The second version does the same as the first but puts in place a load of
tidying up that is permitted by the fix to class array temporaries.



I couldn't readily see how to prepare a testcase - ideas?
Both regtest on FC33/x86_64. The first was tested by Dominique (see the
PR). OK for master?


Typo – underscore-'c' should be a dot-'c' – both changelog files


  * trans-expr_c (gfc_trans_scalar_assign): Make use of pre and


I think the second longer version is nicer in general, but at least for
GCC 9/GCC10 the first version is simpler and, hence, less error prone.

As you only ask about mainline, I would prefer the second one.

However, I am not happy about gfc_find_and_cut_at_last_class_ref:


+ of refs following. If ts is non-null the cut is at the class entity
+ or component that is followed by an array reference, which is not +
an element. */ ... + + if (ts) + { + if (e->symtree + &&
e->symtree->n.sym->ts.type == BT_CLASS) + *ts =
>symtree->n.sym->ts; + else + *ts = NULL; + } + for (ref = e->ref;
ref; ref = ref->next) { + if (ts && ref->type == REF_COMPONENT + &&
ref->u.c.component->ts.type == BT_CLASS + && ref->next &&
ref->next->type == REF_COMPONENT + && strcmp
(ref->next->u.c.component->name, "_data") == 0 + && ref->next->next +
&& ref->next->next->type == REF_ARRAY + && ref->next->next->u.ar.type
!= AR_ELEMENT) + { + *ts = >u.c.component->ts; + class_ref = ref;
+ break; + } + + if (ts && *ts == NULL) + return NULL; +

Namely, if there is:
  type1%array_class2 → array_class2 is used for 'ts' and later (ok)
  type1%type%array_class2 → NULL is returned  (why?)
  class1%type%array_class2 → ts = class1 but array2_class is used later on 
(ups!)
  class1%...%scalar_class2 → ts = class1 but scalar_class2 is used
etc.

Thus this either needs to be cleaned up (separate 'ref' loop for
ts != NULL) – including the wording in the description which tells what
happens if 'ts' is passed as arg but the expr has rank == 0 – and
what value is assigned to 'ts'. (You can then also fix 'class.c::' to
'class.c: ' in the description above the function.)

Alternatively, you can leave the current code ref handling code in place
at build_class_array_ref, which might be the simpler alternative.

Otherwise, it looks sensible to me.

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf


[Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test

2021-03-26 Thread Paul Richard Thomas via Gcc-patches
This patch comes in two versions: submit.diff with Change.Logs or
submit2.diff with Change2.Logs.

The first fixes the problem by changing array temporaries from class
expressions into class temporaries. This permits the use of
gfc_get_class_from_expr to obtain the vptr for these temporaries and all
the good things that come with that when handling dynamic types. The second
part of the fix is to use the array element length from the class
descriptor, when reallocating on assignment. This is needed because the
vptr is being set too early. I will set about trying to track down why this
is happening and fix it after release.

The second version does the same as the first but puts in place a load of
tidying up that is permitted by the fix to class array temporaries.

I couldn't readily see how to prepare a testcase - ideas?

Both regtest on FC33/x86_64. The first was tested by Dominique (see the
PR). OK for master?

Regards

Paul


Change.Logs
Description: Binary data


Change2.Logs
Description: Binary data
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index c6725659093..8aa56d1ccb9 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -1403,9 +1403,6 @@ gfc_trans_create_temp_array (stmtblock_t * pre, stmtblock_t * post, gfc_ss * ss,
   desc = gfc_create_var (type, "atmp");
   GFC_DECL_PACKED_ARRAY (desc) = 1;
 
-  info->descriptor = desc;
-  size = gfc_index_one_node;
-
   /* Emit a DECL_EXPR for the variable sized array type in
  GFC_TYPE_ARRAY_DATAPTR_TYPE so the gimplification of its type
  sizes works correctly.  */
@@ -1416,9 +1413,40 @@ gfc_trans_create_temp_array (stmtblock_t * pre, stmtblock_t * post, gfc_ss * ss,
   gfc_add_expr_to_block (pre, build1 (DECL_EXPR,
   arraytype, TYPE_NAME (arraytype)));
 
-  /* Fill in the array dtype.  */
-  tmp = gfc_conv_descriptor_dtype (desc);
-  gfc_add_modify (pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
+  if (class_expr != NULL_TREE)
+{
+  tree class_data;
+  tree dtype;
+
+  /* Create a class temporary.  */
+  tmp = gfc_create_var (TREE_TYPE (class_expr), "ctmp");
+  gfc_add_modify (pre, tmp, class_expr);
+
+  /* Assign the new descriptor to the _data field. This allows the
+	 vptr _copy to be used for scalarized assignment since the class
+	 temporary can be found from the descriptor.  */
+  class_data = gfc_class_data_get (tmp);
+  tmp = fold_build1_loc (input_location, VIEW_CONVERT_EXPR,
+			 TREE_TYPE (desc), desc);
+  gfc_add_modify (pre, class_data, tmp);
+
+  /* Take the dtype from the class expression.  */
+  dtype = gfc_conv_descriptor_dtype (gfc_class_data_get (class_expr));
+  tmp = gfc_conv_descriptor_dtype (class_data);
+  gfc_add_modify (pre, tmp, dtype);
+
+  /* Point desc to the class _data field.  */
+  desc = class_data;
+}
+  else
+{
+  /* Fill in the array dtype.  */
+  tmp = gfc_conv_descriptor_dtype (desc);
+  gfc_add_modify (pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
+}
+
+  info->descriptor = desc;
+  size = gfc_index_one_node;
 
   /*
  Fill in the bounds and stride.  This is a packed array, so:
@@ -3438,6 +3466,12 @@ build_class_array_ref (gfc_se *se, tree base, tree index)
   && GFC_DECL_SAVED_DESCRIPTOR (se->expr)
   && GFC_CLASS_TYPE_P (TREE_TYPE (GFC_DECL_SAVED_DESCRIPTOR (se->expr
 decl = se->expr;
+  else if (!VAR_P (base) && gfc_get_class_from_expr (base))
+{
+  decl = gfc_get_class_from_expr (base);
+  se->class_vptr = gfc_evaluate_now (gfc_class_vptr_get (decl), >pre);
+  goto class_found;
+}
   else
 {
   if (expr == NULL
@@ -3530,6 +3564,7 @@ build_class_array_ref (gfc_se *se, tree base, tree index)
   if (POINTER_TYPE_P (TREE_TYPE (decl)))
 decl = build_fold_indirect_ref_loc (input_location, decl);
 
+class_found:
   if (!GFC_CLASS_TYPE_P (TREE_TYPE (decl)))
 return false;
 
@@ -10274,23 +10309,10 @@ gfc_alloc_allocatable_for_assignment (gfc_loopinfo *loop,
 }
   else if (expr1->ts.type == BT_CLASS)
 {
-  tmp = expr1->rank ? gfc_get_class_from_expr (desc) : NULL_TREE;
-  if (tmp == NULL_TREE)
-	tmp = gfc_get_class_from_gfc_expr (expr1);
-
-  if (tmp != NULL_TREE)
-	{
-	  tmp2 = gfc_class_vptr_get (tmp);
-	  cond = fold_build2_loc (input_location, NE_EXPR,
-  logical_type_node, tmp2,
-  build_int_cst (TREE_TYPE (tmp2), 0));
-	  elemsize1 = gfc_class_vtab_size_get (tmp);
-	  elemsize1 = fold_build3_loc (input_location, COND_EXPR,
-  gfc_array_index_type, cond,
-  elemsize1, gfc_index_zero_node);
-	}
-  else
-	elemsize1 = TYPE_SIZE_UNIT (gfc_typenode_for_spec (_DATA (expr1)->ts));
+  /* Unfortunately, the lhs vptr is set too early in many cases.
+	 Play it safe by using the descriptor element length.  */
+  tmp = gfc_conv_descriptor_elem_len (desc);
+  elemsize1 = fold_convert (gfc_array_index_type, tmp);
 }
   else
 elemsize1 = NULL_TREE;
@@ -10764,11