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
0001-fortran-implement-conditional-expression-for-fortran.patch
Description: Binary data