From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by sourceware.org (Postfix) with ESMTPS id D5E823857817; Tue, 6 Apr 2021 17:08:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D5E823857817 Received: by mail-lf1-x131.google.com with SMTP id b4so23913549lfi.6; Tue, 06 Apr 2021 10:08:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kysIWGxMxYqhy+OayTuZKMSdoQtcpP8Gf2Es/ZpIbtY=; b=htcPkLMVzj8/dgjAfaGSvgJ4PlxCsjQN20IJEPjiZg9rP0YIlDXzBrvGawYmKghyl6 bLsttMr1/ui5DDjZ2+CkgBdJKwvp/4iKIkak+TXJK9fxS+3bGxFatCKFPNJ60d8EEzoJ aVXN8va2HA+YMagk/8JJ3BrhPSyyWFVUbuSO9k7xn3LZR7yBXyC+rYL8O9QzR+/2X8P4 y+yhP717/c/ER59UXWvTtHJrRq/AqiqmmG6yUPNiwg1K2ESKdGmQIlnAE4F81LeoHK9g 4Q1U+vDjxoSWem/e4b/NTYZVQDv0zMA2wTuFGwbOhlfU6e5tFCZ8K5MAKyg6yIGjCAgT xOqw== X-Gm-Message-State: AOAM532kd3wpU2aQxQKCXal7g4GtOBoNhh0LClo5WwZ7MoypR3cWp4Uy O3mwYonic33U0CkhTesLj53aKfZbgeKTUKcohqjxbE7a5Uk= X-Google-Smtp-Source: ABdhPJz2N+lydAIHGC+wIpbRJq8Ui1uphxyDdM/w9751IVAGHcTeWzEAyTUznwedy/2lFW9KpazAlNBt21sN3YZM9CI= X-Received: by 2002:ac2:4192:: with SMTP id z18mr20800675lfh.19.1617728898365; Tue, 06 Apr 2021 10:08:18 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Paul Richard Thomas Date: Tue, 6 Apr 2021 18:08:06 +0100 Message-ID: Subject: Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test To: Tobias Burnus Cc: "fortran@gcc.gnu.org" , gcc-patches X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Apr 2021 17:08:23 -0000 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 =E2=86=92 NULL is returned (why?) class1%type%array_class2 =E2=86=92 ts =3D class1 but array2_class is use= d later on (ups!) class1%...%scalar_class2 =E2=86=92 ts =3D class1 but scalar_class2 is us= ed 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 al= l > > 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 =E2=80=93 underscore-'c' should be a dot-'c' =E2=80=93 both changelo= g 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 =3D=3D BT_CLASS) + *ts =3D > > &e->symtree->n.sym->ts; + else + *ts =3D NULL; + } + for (ref =3D e->re= f; > > ref; ref =3D ref->next) { + if (ts && ref->type =3D=3D REF_COMPONENT + = && > > ref->u.c.component->ts.type =3D=3D BT_CLASS + && ref->next && > > ref->next->type =3D=3D REF_COMPONENT + && strcmp > > (ref->next->u.c.component->name, "_data") =3D=3D 0 + && ref->next->next= + > > && ref->next->next->type =3D=3D REF_ARRAY + && ref->next->next->u.ar.ty= pe > > !=3D AR_ELEMENT) + { + *ts =3D &ref->u.c.component->ts; + class_ref =3D= ref; > > + break; + } + + if (ts && *ts =3D=3D NULL) + return NULL; + > Namely, if there is: > type1%array_class2 =E2=86=92 array_class2 is used for 'ts' and later (= ok) > type1%type%array_class2 =E2=86=92 NULL is returned (why?) > class1%type%array_class2 =E2=86=92 ts =3D class1 but array2_class is u= sed later > on (ups!) > class1%...%scalar_class2 =E2=86=92 ts =3D class1 but scalar_class2 is = used > etc. > > Thus this either needs to be cleaned up (separate 'ref' loop for > ts !=3D NULL) =E2=80=93 including the wording in the description which te= lls what > happens if 'ts' is passed as arg but the expr has rank =3D=3D 0 =E2=80=93= 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=C3=BCnchen > Registergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thoma= s Heurung, Frank > Th=C3=BCrauf > --=20 "If you can't explain it simply, you don't understand it well enough" - Albert Einstein