From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx-relay24-hz1.antispameurope.com (mx-relay24-hz1.antispameurope.com [94.100.132.224]) by sourceware.org (Postfix) with ESMTPS id EC1A03851C29 for ; Sun, 14 Mar 2021 10:56:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EC1A03851C29 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=net-b.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=prvs=0700b8c1b2=burnus@net-b.de Received: from s041.wsp.plusnet.de ([195.90.7.81]) by mx-relay24-hz1.antispameurope.com; Sun, 14 Mar 2021 11:56:47 +0100 Received: from [192.168.8.102] (tmo-117-141.customers.d1-online.com [80.187.117.141]) by s041.wsp.plusnet.de (Postfix) with ESMTPSA id 08BA22C0112; Sun, 14 Mar 2021 11:56:43 +0100 (CET) Subject: Re: [PATCH] PR fortran/99112 - [11 Regression] ICE with runtime diagnostics for SIZE intrinsic function To: Paul Richard Thomas , Harald Anlauf Cc: gcc-patches , fortran References: From: Tobias Burnus Message-ID: Date: Sun, 14 Mar 2021 11:56:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 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-relay24-hz1.antispameurope.com with 96402124093C X-cloud-security-connect: s041.wsp.plusnet.de[195.90.7.81], TLS=1, IP=195.90.7.81 X-cloud-security-Digest: 16793d5d79ce9339c12224d4342788fc X-cloud-security: scantime:1.463 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, 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: Sun, 14 Mar 2021 10:56:51 -0000 Hi Harald, hi Paul, On 13.03.21 09:58, Paul Richard Thomas via Fortran wrote: > I am not sure of the etiquette for this - it looks OK to me :-) :-) On Fri, 12 Mar 2021 at 21:20, Harald Anlauf via Fortran > the addition of runtime checks for the SIZE intrinsic created a regression > that showed up for certain CLASS arguments to procedures. Paul did most of > the work (~ 99%), but asked me to dig into an issue with an inappropriately > selected error message. This actually turned out to be a simple one-liner > on top of Paul's patch. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? The procedure-call patch looks good to me, except for: > --- a/gcc/fortran/trans-expr.c > +++ b/gcc/fortran/trans-expr.c > @@ -6662,6 +6662,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, > [...] > + tree temp; > ... > - tmp = parmse.expr; > + if (fsym && fsym->ts.type == BT_CLASS) > + { > + temp = build_fold_indirect_ref_loc (input_location, > ... > + else > + temp = parmse.expr; > ... > - if (!POINTER_TYPE_P (TREE_TYPE (parmse.expr))) > - tmp = gfc_build_addr_expr (NULL_TREE, tmp); > + if (!POINTER_TYPE_P (TREE_TYPE (temp))) > + temp = gfc_build_addr_expr (NULL_TREE, temp); > ... > - logical_type_node, tmp, > - fold_convert (TREE_TYPE (tmp), > + logical_type_node, temp, > + fold_convert (TREE_TYPE (temp) I do not see any reason why 'tmp' is replaced by 'temp' in this code. Also for doing patch archeology, it helps if there are no changes unless it makes sense. Adding an -e- does not count ;-) Hence, OK with that change. * * * Regarding the change: gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr) It looks as if the pointer/check is done for size(dt%foo%bar) for the 'bar' component' but shouldn't it also be done for each part ref, if it is a pointer/allocatable? (i.e. 'dt', 'foo' and 'bar'? That's independent of the current patch. Additionally, as there are a lot of special cases for CLASS – I wonder whether there also needs to be a special case for size(dt%foo%class) ? In particular, the following does ICE for me: module m type t class(*), pointer :: bar(:) end type type t2 class(t), allocatable :: my(:) end type t2 contains function f (x, y) result(z) class(t) :: x(:) class(t) :: y(size(x(1)%bar)) type(t) :: z(size(x(1)%bar)) end function g (x) result(z) class(t) :: x(:) type(t) :: z(size(x(1)%bar)) end subroutine s () class(t2), allocatable :: a(:), b(:), c(:), d(:) class(t2), pointer :: p(:) c(1)%my = f (a(1)%my, b(1)%my) d(1)%my = g (p(1)%my) end end * * * > P.S.: I couldn't find a Changelog entry that uses co-authors. Is the > version > below correct? ... > Co-authored-by: Paul Thomas I think you have two options: either the GIT way – as you did (although I think the GIT way usually only has one and not two spaces before the email). I did not see it in the commit logs, but it is used in the testcases for the change-log generator, see: contrib/gcc-changelog/test_patches.txt Alternative way is to specify the authors in the classical ChangeLog style; latest real-world example is https://gcc.gnu.org/g:d656bfda2d8316627d0bbb18b10954e6aaf3c88c but you can also look at the contrib/gcc-changelog/test_patches.txt Finally, you can also run the script at contrib/gcc-changelog/ yourself as pre-commit check. For instance, if you committed but did not yet push it: git show -1 | ./contrib/gcc-changelog/git_check_commit.py -p to errors there, you can use 'git commit --amend' until you have pushed that commit (then it is too late). Tobias