From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx-relay99-hz2.antispameurope.com (mx-relay99-hz2.antispameurope.com [94.100.136.199]) by sourceware.org (Postfix) with ESMTPS id C3FA03886C7C for ; Sat, 10 Apr 2021 18:10:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C3FA03886C7C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=net-b.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=prvs=07276eb82d=burnus@net-b.de Received: from s041.wsp.plusnet.de ([195.90.7.81]) by mx-relay99-hz2.antispameurope.com; Sat, 10 Apr 2021 20:10:08 +0200 Received: from [192.168.0.34] (port-92-195-249-203.dynamic.as20676.net [92.195.249.203]) by s041.wsp.plusnet.de (Postfix) with ESMTPSA id EDE952C026C; Sat, 10 Apr 2021 20:10:04 +0200 (CEST) Subject: Re: [Patch, fortran] 99307 - FAIL: gfortran.dg/class_assign_4.f90 execution test To: Paul Richard Thomas , Tobias Burnus Cc: gcc-patches , "fortran@gcc.gnu.org" References: From: Tobias Burnus Message-ID: <7ae40bbe-54dc-09d1-027a-4347390819dd@net-b.de> Date: Sat, 10 Apr 2021 20:10:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-cloud-security-sender: burnus@net-b.de X-cloud-security-recipient: fortran@gcc.gnu.org X-cloud-security-Virusscan: CLEAN X-cloud-security-disclaimer: This E-Mail was scanned by E-Mailservice on mx-relay99-hz2.antispameurope.com with 6D15BE060BB X-cloud-security-connect: s041.wsp.plusnet.de[195.90.7.81], TLS=1, IP=195.90.7.81 X-cloud-security-Digest: cc75facd6d4fca09370d152a48da4dbb X-cloud-security: scantime:1.676 X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: Sat, 10 Apr 2021 18:10:12 -0000 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 = >>> &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 >> >