From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 899113858D38; Tue, 20 Jun 2023 08:11:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 899113858D38 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1687248679; bh=dviKO/d58TuIII6JCOlElgcqMuHBZtgLctQEwLItpu0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=R4jJ0eSTNhQWrap+B0ptlQpX7/qNtFWW5lj0R/jfoHfhhA3XjQk+p/rhKWVFqH1We wS30ZnMBrDdTslIv7rkWLtEVUHd+uQCXKOKHfiSefUDL9fflMN6kHUdP8We/MAZhBz yfEypWRCpnVMPC7wwnnIwhrZkZC12x+uBd0ga3pA= From: "mikael at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug fortran/92887] [F2008] Passing nullified/disassociated pointer or unalloc allocatable to OPTIONAL + VALUE dummy fails Date: Tue, 20 Jun 2023 08:11:16 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: fortran X-Bugzilla-Version: 10.0 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: mikael at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D92887 --- Comment #7 from Mikael Morin --- (In reply to anlauf from comment #6) > (In reply to Mikael Morin from comment #5) > > (In reply to anlauf from comment #4) > > > @@ -6396,7 +6399,28 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symb= ol * sym, > > > && fsym->ts.type !=3D BT_CLASS > > > && fsym->ts.type !=3D BT_DERIVED) > > > { > > > - if (e->expr_type !=3D EXPR_VARIABLE > > > + /* F2018:15.5.2.12 Argument presence and > > > + restrictions on arguments not present. */ > > > + if (e->expr_type =3D=3D EXPR_VARIABLE > > > + && (e->symtree->n.sym->attr.allocatable > > > + || e->symtree->n.sym->attr.pointer)) > >=20 > > Beware of expressions like derived%alloc_comp or derived%pointer_comp w= hich > > don't match the above. >=20 > Right. This is fixable by using >=20 >=20 > && (gfc_expr_attr (e).allocatable > || gfc_expr_attr (e).pointer)) >=20 > instead. >=20 Sure. Easy. :-) > > > @@ -7072,6 +7096,42 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symb= ol * sym, > > > } > > > } > > >=20=20 > > > + /* F2023:15.5.3, 15.5.4: Actual argument expressions are evalu= ated > > > + before they are associated and a procedure is executed. */ > > > + if (e && e->expr_type !=3D EXPR_VARIABLE && !gfc_is_constant_e= xpr (e)) > > > + { > > > + /* Create temporary except for functions returning pointers that > > > + can appear in a variable definition context. */ > >=20 > > Maybe explain *why* we have to create a temporary, that is some data > > references may become undefined by the procedure call (intent(out) dum= mies) > > so we have to evaluate values depending on them beforehand (PR 92178). >=20 > That is one reason. Another one, also pointed out in PR92178 by Tobias' > review > of Steve's draft, is the first testcase at >=20 > https://gcc.gnu.org/legacy-ml/gcc-patches/2019-10/msg01970.html >=20 Not sure I understand. Looks like the same reason to me. Or maybe my words are not clear enough. > This is reminiscent to an issue reported for the MERGE intrinsic (pr10787= 4, > fixed so far, but there is a remaining issue in pr105371). >=20 I don't see how pr107874 or pr105371 are related to this. > > > + need_temp =3D true; > > > + } > > > + > > > + if (need_temp) > > > + { > > > + if (cond_temp =3D=3D NULL_TREE) > > > + parmse.expr =3D gfc_evaluate_now (parmse.expr, &parmse.pre); > >=20 > > I'm not sure about this. The condition to set need_temp looks quite ge= neral > > (especially it matches non-scalar cases, doesn't it?), but > > gfc_conv_expr_reference should already take care of creating a variable= , so > > that a temporary is missing only for value dummies, I think. I would r= ather > > move this to the place specific to value dummies. >=20 > I agree in principle. The indentation level is already awful in the spec= ific > place, which calls for thoughts about refactoring that mega-loop over the > arguments than currently spans far more than 1000 source code lines. >=20 Yes. The situation is severely out of hand there. We could just outline t= he for loop body to a separate function, but I'm not sure it would by us much. We could use different classes defining the argument passing convention (by reference, by value with an extra argument, with array container, with class container, etc) and dispatch to the methods of those classes. But it's difficult to have a global understanding of what is needed here.=20 > > > + else > >=20 > > I would rather move the else part to the place above where cond_temp is= set, > > so that the code is easier to follow. > >=20 > > > + { > > > + /* "Conditional temporary" to handle variables that possibly > > > + cannot be dereferenced. Use null value as fallback. */ > > > + tree dflt_temp; > > > + gcc_assert (e->ts.type !=3D BT_DERIVED && e->ts.type !=3D BT_= CLASS); > > > + gcc_assert (e->rank =3D=3D 0); > > > + dflt_temp =3D gfc_create_var (TREE_TYPE (parmse.expr), "temp"= ); > > > + TREE_STATIC (dflt_temp) =3D 1; > > > + TREE_CONSTANT (dflt_temp) =3D 1; > > > + TREE_READONLY (dflt_temp) =3D 1; > > > + DECL_INITIAL (dflt_temp) =3D build_zero_cst (TREE_TYPE (dflt_= temp)); > > > + parmse.expr =3D fold_build3_loc (input_location, COND_EXPR, > > > + TREE_TYPE (parmse.expr), > > > + cond_temp, > > > + parmse.expr, dflt_temp); > > > + parmse.expr =3D gfc_evaluate_now (parmse.expr, &parmse.pre); > > > + } > > > + } > > > + > > > + > > > if (fsym && need_interface_mapping && e) > > > gfc_add_interface_mapping (&mapping, fsym, &parmse, e); > > > >=20 > I agree that it was a bad idea to merge the patches for these two PRs just > because temporaries are needed. Well, the structure of the existing code doesn't really help to guide chang= es.=20 I thing we should try to make it possible to thing locally about a specific case without caring about the rest. But as new missing cases are discovere= d we keep adding code, making it less and less local, and ... Well, we keep feed= ing the monster.=