Hi Yuao,
Yuao Ma wrote:
Hi Tobias,
I have some updates about this patch.
First, some good news:
1. The patch has been bootstrapped and tested with no regressions.
This was achieved by limiting the type with only one 'sorry' case.
2. The frontend parsing now considers outer parentheses.
:-)
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.)
* * *
And we also have some unresolved problems.
1. First is the use of a conditional expression as an argument.
Consider the following code example: it will compile but produce an
incorrect result. The reason is that the current implementation passes
the address of a local output to the function, rather than the address
of the true_expr or false_expr.
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. Note
that for 'OPTIONAL, VALUE' the actual argument is passed by value - but
there is a hidden Boolean/logical argument which indicates whether the
argument is present. - Here, also '(cond ? a : b)' should work, i.e.
only when the respective variable is present it should show up as
present - here 'a' or 'b'. [I think before Fortran 2018, only scalars
(with character length == 1) were supported with VALUE, but since
Fortran 2018 also, e.g., arrays are supported. - Those internally must
be passed by reference, i.e. a copy has to be created at some pointer
(caller presumably makes most sense) - an then no hidden argument is
required. I have not rechecked, but I think gfortran does not support
this case, yet.] * * *
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.
I hope that something like se.want_pointer will solve most issues as
there are many code paths like for intrinsics like 'lbound(cond ? a : b)'
or even 'c_loc(cond ? a : b)' which act like functions to the users but
have special code as being an intrinsic (mainly in trans-instrinsic.cc).
For user-written procedures: those are handled via
trans-expr.cc's gfc_conv_procedure_call.
* * *
+ /* TODO: support more data types for conditional expressions */
+ if (true_expr->ts.type != BT_INTEGER && true_expr->ts.type != BT_LOGICAL
+ && true_expr->ts.type != BT_REAL)
+ {
+ gfc_error ("Sorry, conditional expression at %L currently only support "
+ "integer, logical, and real types",
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.
Tobias,
who still has to re-read the patch again.
