public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Paul Richard Thomas <paul.richard.thomas@gmail.com>
To: Tobias Burnus <tobias@codesourcery.com>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test
Date: Tue, 6 Apr 2021 18:08:06 +0100	[thread overview]
Message-ID: <CAGkQGiJqnmX7Cs=P-xqJLgLNahsYyC85J7i6W3H+jr1RHyZZ8A@mail.gmail.com> (raw)
In-Reply-To: <b5bba6a5-a3aa-0ee2-8434-1bfae680306e@codesourcery.com>

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 <tobias@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

  reply	other threads:[~2021-04-06 17:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-26 16:30 Paul Richard Thomas
2021-03-29 13:58 ` Tobias Burnus
2021-04-06 17:08   ` Paul Richard Thomas [this message]
2021-04-10 18:10     ` Tobias Burnus
2021-04-11  7:05     ` Paul Richard Thomas
2021-04-14 13:51       ` Tobias Burnus
2021-04-15 12:02         ` Paul Richard Thomas
2021-03-26 17:29 dhumieres.dominique
2021-03-26 18:20 ` Paul Richard Thomas
2021-03-27 14:04   ` dhumieres.dominique

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGkQGiJqnmX7Cs=P-xqJLgLNahsYyC85J7i6W3H+jr1RHyZZ8A@mail.gmail.com' \
    --to=paul.richard.thomas@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=tobias@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).