Pushed to master in commit 9a0e09f3dd5339bb18cc47317f2298d9157ced29 Thanks
Paul On Wed, 14 Apr 2021 at 14:51, Tobias Burnus <tob...@codesourcery.com> 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 > > <paul.richard.tho...@gmail.com <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 > > <tob...@codesourcery.com <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 = > > > &e->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 = &ref->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 > ----------------- > 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
