Hi Tobias,

On Tue, Sep 2, 2025 at 4:57 PM Tobias Burnus <tbur...@baylibre.com> wrote:
> BTW: I notice '(Stripping trailing CRs from patch; use --binary to disable.)'
> when applying the patch. - Spurious '\r' in committed patches should be
> avoided, albeit testcases with '\r\n' are fine as we also need to support
> them on Windows. (Hence, testsuite pattern that check for linebreaks also
> must handle them.)

This time I use git format-patch. Hopefully fix the issue.

> >> Based on our previous discussion, we
> >> don't need to necessarily handle this in the current patch; I just
> >> wanted to highlight it. I will investigate how the argument-passing
> >> logic works.
> >
> > Yes, as long as there is a 'sorry' instead of producing wrong code,
> > it is fine. But I would like to avoid seeing runtime fails.
>
> BTW: When thinking about the item below, I think the following
> needs to have a condition:
>
>    /* Match an expression in parentheses.  */
>    if (gfc_match_char ('(') != MATCH_YES)
> ...
>    m = match_conditional (&e);
> ...
>    m = gfc_match_char (')');
> ...
>    /* Now we have the expression inside the parentheses, build the
>       expression pointing to it. By 7.1.7.2, any expression in
>       parentheses shall be treated as a data entity.  */
>    *result = gfc_get_parentheses (e);
>
> Namely, if 'e->expr_type == EXPR_CONDITIONAL, the extra parentheses
> should not be added.
>

Agreed. Fixed.

> * * *
>
> I think something like the following would be useful during
> parsing:
>
> +  gfc_gobble_whitespace ();
> +  locus saved_loc = gfc_current_locus;
>     m = gfc_match_expr (&true_expr);
> +  if (m != MATCH_YES && (m = gfc_match (".nil.")) == MATCH_YES)
> +    true_expr = gfc_get_..._expr (saved_loc, ... )
>     if (m != MATCH_YES)
>
> To parse the .nil.; I think in the first round, we should reject
> it unconditionally during resolving.
>
> I am not completely sure how to store it, but I was wondering
> whether it should be added to operator (EXPR_OP), which is contrary
> to the others does not take any expression. As it should only be
> reachable by resolving the conditional operation, it shouldn't
> harm elsewhere but needs to be handled everywhere where
> conditional is processed. During parsing, it should be parsed after
> other options failed while during resolution/dumping, it needs to
> be handled before as, e.g., resolve_operator does not know about it
> (nor does it need to, IMHO).
>
> * * *
>
> In any case, we later have to handle the three cases:
> - accept it for actual arguments to OPTIONAL dummy arguments
> - reject it for dummy arguments that are not OPTIONAL
> - reject it when not used as actual argument.
>
> I wonder whether we can just reject it unconditionally,
> except when explicitly processed as actual argument.
> Thus, in resolve_conditional, we could rejected in general.
>
> When processing actual arguments, only expr == EXPR_CONDITIONAL
> with either argument being .NIL. makes sense (or the first/second
> being again a EXPR_CONDITIONAL). Thus, those could be explicitly
> handled for the OPTIONAL check. To mark the .NIL. and to be not
> diagnosed, I think we can just set the expr->do_not_resolve_again = 1
> in the argument handling - and check for it with a comment in
> resolve_conditional, skipping the .NIL. diagnostic only
> in that case.
>
> The only caveat is that there are many ways to call procedures
> (functions and subroutines, specific or generic, type-bound or
> procedure pointers), including intrinsic procedures, which are
> resolved slightly differently. - I am not sure how many call paths
> remain that need to be checked, hopefully not many!
> In several cases, the normal diagnostic will do as not all take
> optional arguments, but some – including intrinsic procedures -
> do.
>
> In any case, I think we should soon handle .NIL., but first reject
> it unconditionally.
>

Thanks for the thorough analysis! I haven't had a chance to look into
the nil part yet (I'll probably get to it tomorrow), specifically from
the parsing to the semantic analysis phase. Now using nil in a
conditional expression would result in an immediate error. Some kind
of coherent lol.

> If you add a 'VALUE' (pass by value) and, possibly, remove OPTIONAL – it
> should work. That enforces pass by value; if you use pass by reference
> semantic, there is an issue. I think if the expression needs a pointer,
> you could just move the pointer-ness on, i.e. instead of '&(cond ? a :
> b)' you produce '(cond ? &a : &b)' to use pseudo-C. I wonder whether
> playing with se.want_pointer is enough or whether you need more.

That approach works. My current method is to handle EXPR_CONDITIONAL
in gfc_conv_expr_reference by setting want_pointer=1. Then, in
trans_expr, I use this want_pointer value to determine if the
true_expr and false_expr need to be pointers. I've added a test case,
conditional_6.f90.

> I think there is no reason to not permit BT_COMPLEX as well; all of those
> are numerical types, which would be an alternative to listening all 
> explicitly.
>
> I think the passive voice, using "currently, only ... are supported", might
> be clearer. At least to me, the current wording can be read such that
> that's a property of of the conditional expressions and not a limitation
> of the compiler.

Both are fixed and tested.

Thanks,
Yuao

Attachment: 0001-fortran-implement-conditional-expression-for-fortran.patch
Description: Binary data

Reply via email to