From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.smtpout.orange.fr (smtp03.smtpout.orange.fr [80.12.242.125]) by sourceware.org (Postfix) with ESMTPS id D3CDA3858D35 for ; Thu, 25 Nov 2021 21:02:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D3CDA3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orange.fr Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orange.fr Received: from [192.168.1.17] ([92.167.144.168]) by smtp.orange.fr with ESMTPA id qLswmfuHzIEdlqLswmOSVA; Thu, 25 Nov 2021 22:02:27 +0100 X-ME-Helo: [192.168.1.17] X-ME-Auth: MDU4MTIxYWM4YWI0ZGE4ZTUwZWZmNTExZmI2ZWZlMThkM2ZhYiE5OWRkOGM= X-ME-Date: Thu, 25 Nov 2021 22:02:27 +0100 X-ME-IP: 92.167.144.168 Subject: Re: [PATCH] PR fortran/103411 - ICE in gfc_conv_array_initializer, at fortran/trans-array.c:6377 To: Harald Anlauf , fortran , gcc-patches References: <4fb65fbe-c10f-3fd4-9961-9978ff386bf9@orange.fr> <24ff1ac7-79ce-2843-a199-9e85865304f6@gmx.de> From: Mikael Morin Message-ID: <14808971-a1a7-4742-7a06-7d9102bf2b24@orange.fr> Date: Thu, 25 Nov 2021 22:02:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <24ff1ac7-79ce-2843-a199-9e85865304f6@gmx.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FROM, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Nov 2021 21:02:30 -0000 Le 25/11/2021 à 21:03, Harald Anlauf a écrit : > Hi Mikael, > > Am 25.11.21 um 17:46 schrieb Mikael Morin: >> Hello, >> >> Le 24/11/2021 à 22:32, Harald Anlauf via Fortran a écrit : >>> diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c >>> index 5a5aca10ebe..837eb0912c0 100644 >>> --- a/gcc/fortran/check.c >>> +++ b/gcc/fortran/check.c >>> @@ -4866,10 +4868,17 @@ gfc_check_reshape (gfc_expr *source, gfc_expr >>> *shape, >>>      { >>>        gfc_constructor *c; >>>        bool test; >>> +      gfc_constructor_base b; >>> >>> +      if (shape->expr_type == EXPR_ARRAY) >>> +        b = shape->value.constructor; >>> +      else if (shape->expr_type == EXPR_VARIABLE) >>> +        b = shape->symtree->n.sym->value->value.constructor; >> >> This misses a check that shape->symtree->n.sym->value is an array, so >> that it makes sense to access its constructor. > > there are checks further above for the cases >   shape->expr_type == EXPR_ARRAY > and for >   shape->expr_type == EXPR_VARIABLE > which look at the elements of array shape to see if they are > non-negative. > > Only in those cases where the full "if ()'s" pass we set > shape_is_const = true; and proceed.  The purpose of the auxiliary > bool shape_is_const is to avoid repeating the lengthy if's again. > Only then the above cited code segment should get executed. > > For shape->expr_type == EXPR_ARRAY there is really no change in logic. > For shape->expr_type == EXPR_VARIABLE the above snipped is now executed, > but then we already had > >   else if (shape->expr_type == EXPR_VARIABLE && shape->ref >        && shape->ref->u.ar.type == AR_FULL && shape->ref->u.ar.dimen == 1 >        && shape->ref->u.ar.as >        && shape->ref->u.ar.as->lower[0]->expr_type == EXPR_CONSTANT >        && shape->ref->u.ar.as->lower[0]->ts.type == BT_INTEGER >        && shape->ref->u.ar.as->upper[0]->expr_type == EXPR_CONSTANT >        && shape->ref->u.ar.as->upper[0]->ts.type == BT_INTEGER >        && shape->symtree->n.sym->attr.flavor == FL_PARAMETER >        && shape->symtree->n.sym->value) > > In which situations do I miss anything new? > Yes, I agree with all of this. My comment wasn’t about a check on shape->expr_type, but on shape->value->expr_type if shape->expr_type is a (parameter) variable. >> Actually, this only supports the case where the parameter value is >> defined by an array; but it could be an intrinsic call, a sum of >> parameters, a reference to an other parameter, etc. > > E.g. the following (still) does get rejected: > >   print *, reshape([1,2,3,4,5], a+1) >   print *, reshape([1,2,3,4,5], a+a) >   print *, reshape([1,2,3,4,5], 2*a) >   print *, reshape([1,2,3,4,5], [3,3]) >   print *, reshape([1,2,3,4,5], spread(3,dim=1,ncopies=2)) > > and has been rejected before. > >> The usual way to handle this is to call gfc_reduce_init_expr which (pray >> for it) will make an array out of whatever the shape expression is. > > Can you give an example where it fails? > > I think the current code would almost certainly fail, too. > Probably, I was just trying to avoid followup bugs. ;-) I have checked the following: integer, parameter :: a(2) = [1,1] integer, parameter :: b(2) = a + 1 print *, reshape([1,2,3,4], b) end and it doesn’t fail as I thought it would. So yes, I was wrong; b has been expanded to an array before. Can you add an assert or a comment saying that the parameter value has been expanded to a constant array? Ok with that change.