From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by sourceware.org (Postfix) with ESMTPS id BD4523858CDA; Fri, 7 Oct 2022 18:46:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BD4523858CDA Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1665168381; bh=5wGtHR8m8Up96Tkv5D2GtYI+oIQXOPFb5jEpx81J2EY=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=CJf1fKhmM/QvsLC5cPnEaL1GJLRyri+f6kcJGWDZZE9solyhnsvld3PUbMoOF88wK 5Qmc/U/WJ5+Se4KDZSo+SPs0Z8bRoN4vnUVmSNokJFMQFJXHIrb05KdPW43goUh3c7 sjxkZImWyLGHPaZOkfJG6RNwGNm3OxwY0Jn+1+a4= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.178.29] ([93.207.84.166]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1M4axg-1oiWmx2KaE-001iBt; Fri, 07 Oct 2022 20:46:21 +0200 Message-ID: <05f415c0-80bc-c04e-a142-1251bf82bb1d@gmx.de> Date: Fri, 7 Oct 2022 20:46:20 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH, v2] Fortran: error recovery for invalid types in array constructors [PR107000] To: Mikael Morin Cc: gcc-patches , fortran Newsgroups: gmane.comp.gcc.fortran,gmane.comp.gcc.patches References: <1bf3b7b5-39ac-0c94-256c-f739a4746a7b@orange.fr> <97dd508f-83b0-5ed0-8cb5-f4f7c8fe08e6@orange.fr> <05a23138-adcd-2526-698c-1fa846f1810b@orange.fr> Content-Language: en-US From: Harald Anlauf In-Reply-To: <05a23138-adcd-2526-698c-1fa846f1810b@orange.fr> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:+MMwrufSLMCqmYimdDWwv9cwqOUtGjTuJuQV8nvP5ki8Klfxnz1 KiBl3yZ3Aq9YwiJkVq2yu9NMDQdOYq52UVdaoDuKrDEs2DqgLuVajCRd55854f7a1RJsQun 6osV7RGY5WQY5KIJJZjCWRrdbOAkDE29jfOSbSZQBOcfwJEai1FnrPH5YcGwD4HeBiMofgS YJMJbu+RkWUYVj1JlruDA== X-UI-Out-Filterresults: notjunk:1;V03:K0:+TdLwfrMy50=:3MMQelsFpYOBX2ftfwDeds V9Dvpn5C3DATi5R4Hw+KedIf1/4nxtwT/9PPH0T1r7cMMZ2oohd1e6q6tF0+8ckbeayylyWbf v6newmq1yNqhGZSGPi4EUry2phNbXR5IUrXe2j4KDTcsqiJfYZz7MXpfNB3GsXDAPwGMUPUpH pp1WRiibyu6L2q2XXnKrPivqoCQJgOprO2sGPMG+agkyFqx6F3fiyAHs/PVab68kRUfCnqjtM vDvcSI8zyIn0isXK1lX0ZTwFRbT+BX+mcJ/aDXMjP4H31tnfsTlGmgizddn2HB5RsIYUnku3A NHjzM75OdDq2DOnibnlKOKsM+e0mvjhj2rq6InaxEv1LZJt02XrIBFIIwysAYQpu2hueBCNJR qnfHgzM+U+/8OsRiT7KpMP2oZTKIb50SJADCqcEFDSii+qeowZ7RSLf0lZrZoWSBlnLNuIBgO 9+qs6S115DxU6ynthGcjvA7DCwHuI4CvDmrmsAOf/1xOT6N6raRd0HHF8+pqgUzC0RIJATUdH 5rEyzWFoFVvgdf2227ffCzbQzcd1lFqFGgSA/h/inbnbOws9WsOQ3nsjspBkrBEM3kuS9P1QK fsffLP7iYday5/KEnpUHFw4FCsAqfZXKyUiYvYOQFP/HsV1vimh0WJd8wrkU58ZwfDaPcAgTu l/F5V0hZRJYC722kaj5PcjPAG3vn0OOTvoj6DJrpPLDp8aI1eB9DX7/c2GU/NbxcexYdYXM6h wSz1Gb8VDNvs4pulax0K0skjIL1+EmssDlMXeUedlfaz8k8mLCxVuA2FCaLiKJO6ePO3mFoOr SwJ9926NKm03QmEndekzOq44D2i8jYjpUMzJYetbBxthLCj0FMLCqRN1ENUlcAv4b/6n72sq6 dKp/Z7U/u0KI510yx4tDOG1u9pBFGX5NLSxjVywlMzLcVzfVk7GsCwItJahFodGbgLkOh2CFI fTPPUMz1/rVi6JZtIphbcadTn7JcOJItTBpv07952IB/tkaRNuILX3ztzO76gqMSfbuTzvwSV Um3CsFlwtlYMIoU6KGl/aPEKJNCC4Ut2ZLJVBZbtaqGz/d7pajjYPC51aPIdpLZDN0RxK2dys vASIzgL+pwf9NmQFJLUuNDGS84ZavLlHVA5zOoEyXIuTaZX/xXLteKFhf8Vx/6tO540yfzkRP 6MInhNtMJN8hWWvPaHY+vDCN/kYcGM2ETbpQbYg4fv+jJyfxhq90s9izXRsgoFzdz/hwoBPfD ULTUn17KcvDkJdPoIOEQhXmpEkx3Dv+PWp9S3KoXN868ZGzrmLktq6ST4S8mW8ae9MKZEX7jX wjKPR+Yw8lqKrr90o7b0H7xYkUYJi/wLoCflIIDaBIHcKmqif/OZBAnhoSA0eEVd0N+YyKofd 5TFKcYBRPapo80zfjNzXPDz7ZFSDva8xobHTej3SNu02uOji1uUB2uEAgl3kklfO8Qwz6qnss vwUpr1FsND/xoDNd/XNO95mwSmTNOdu14qr/ZbbxmbfWAUjbSQhmy0oxJaFIhuFQAmT7F5duu JJ9B1tsSzf9rbfQ8IvSijcu0PUbXeZfxpABrSAZDNcpz5 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Am 07.10.22 um 10:01 schrieb Mikael Morin: > Le 06/10/2022 =C3=A0 23:36, Harald Anlauf a =C3=A9crit=C2=A0: >>> >>> For example, for this case: >>> >>> [real :: 2] * [real :: +(.true.)] >>> >>> First there is a "root" invocation of reduce binary with arguments [re= al >>> :: 2] and [real :: +(.true.)] >>> The root invocation of reduce_binary will call reduce_binary_aa. This = is >>> normal. >>> >>> Then reduce_binary_aa calls reduce_binary again with arguments 2 and >>> +(.true.).=C2=A0 And reduce_binary calls again reduce_binary_aa with t= hose >>> arguments.=C2=A0 This is weird, reduce_binary_aa is supposed to have a= rrays >>> for both arguments. >> >> Am I seeing something different from you?=C2=A0 My gdb says >> that one argument of reduce_binary is EXPR_CONSTANT, >> the other EXPR_OP and BT_UNKNOWN.=C2=A0 Both rank 0. >> > No, I get the same, and the program goes to reduce_binary_aa with those > arguments; this is the problem. > >>> The same goes for the array vs constant case, reduce_binary_ca (or >>> reduce_binary_ac) is invoked with two scalars, while if you look at >>> reduce_binary, you would expect that we only get to reduce_binary_ca >>> with a scalar constant and an array as arguments. >>> >>> >>> I think the checks in the three reduce_binary_* functions should be >>> moved into their respective loops, so that we detect the invalid type >>> just before these weird recursive calls instead of just after entering >>> into them. >> >> I think I tried that before, and it didn't work. >> There was always one weird case that lead to a bad or >> invalid constructor for one of the arrays you want to >> look at in the respective loop,=C2=A0 and this is why the >> testcase tries to cover everything that I hit then and >> there... (hopefully).=C2=A0 So I ended up with the check >> before the loop. >> > I see, I'll have a look. > >> What do we actually gain with your suggested change? >> Moving the check into the loop does not really make >> the code more readable to me.=C2=A0 And the recursion is >> needed anyway. >> > I think we gain clarity, consistency. > > I try to rephrase again. > From a high level point of view, to evaluate a binary operator you need > a specific (one for each operator) function to evaluate the scalar vs > scalar case, and three generic (they are common to all the operators) > functions to handle respectively: > =C2=A0- the scalar vs array case, > =C2=A0- the array vs scalar case, > =C2=A0- the array vs array case, > by calling in a loop the scalar specific function. > Here we are only dealing with constants, arrays of constants, arrays of > arrays, etc, all valid cases. > > Your patch introduces support for invalid cases, that is invalid values > that can't be reduced to a constant.=C2=A0 This is fine, and it works. > What is weird is that the scalar vs invalid scalar case is caught in the > array vs array function. OK, that is because reduce_binary dispatches the reduce_binary_*. We could move the check from reduce_binary_aa to the beginning of reduce_binary, as with the following change on top of the patch: diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index 2c57c796270..91e70655ad3 100644 =2D-- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -1426,10 +1426,6 @@ reduce_binary_aa (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **), if (!gfc_check_conformance (op1, op2, _("elemental binary operation"))= ) return ARITH_INCOMMENSURATE; - if ((op1->expr_type =3D=3D EXPR_OP && op1->ts.type =3D=3D BT_UNKNOWN) - || (op2->expr_type =3D=3D EXPR_OP && op2->ts.type =3D=3D BT_UNKNOWN= )) - return ARITH_INVALID_TYPE; - head =3D gfc_constructor_copy (op1->value.constructor); for (c =3D gfc_constructor_first (head), d =3D gfc_constructor_first (op2->value.constructor); @@ -1467,6 +1463,10 @@ static arith reduce_binary (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **), gfc_expr *op1, gfc_expr *op2, gfc_expr **result) { + if ((op1->expr_type =3D=3D EXPR_OP && op1->ts.type =3D=3D BT_UNKNOWN) + || (op2->expr_type =3D=3D EXPR_OP && op2->ts.type =3D=3D BT_UNKNOWN= )) + return ARITH_INVALID_TYPE; + if (op1->expr_type =3D=3D EXPR_CONSTANT && op2->expr_type =3D=3D EXPR_= CONSTANT) return eval (op1, op2, result); However, we cannot remove the checks from reduce_binary_ac or reduce_binary_ca, as the lengthy testcase proves... Do you like the above better? Cheers, Harald