Yuao Ma wrote:
And the latter does not handle EXPR_CONDITIONAL.
Thanks for the tip! It turns out that not only does gfc_traverse_expr
fail to handle conditional expressions, but check_restricted and
gfc_check_init_expr don't either. I've added all the necessary fixes,
and the test case is now in conditional_7.f90.
(Not sure why some of these weren't caught by -Wswitch)
Because we have "default: gcc_unreachable ();"
In cases where all variants should explicitly handled,
I am personally very much in favor of removing the "default"
and letting -Werror=switch handle the rest.
I think that would work as - to my knowledge - the bootstrap
build enables this -Werror=. On the other hand, for non-bootstrap
builds¹ or for cases where only a few cases should ever reach the
switch, using "default: gcc_unreachable ();" is better – without
it would just fall through which is worse than a runtime fail.
(¹ For non-bootstrap builds, we cannot enable -Werrors as there
is no control about what the system compiler will warn about.
For bootstrap's stage2/stage3, it is the current GCC compiler
itself, which makes it a well-defined version.)
* * *
+ if (true_expr->rank != false_expr->rank)
+ {
+ gfc_error ("true_expr at %L and false_expr at %L in conditional "
+ "expression must have the same rank",
+ &true_expr->where, &false_expr->where);
+ return false;
+ }
I wonder whether 'true_expr' / 'false_expr' are the best wording; Fortran
itself just has:
"R1002 conditional-expr is ( scalar-logical-expr ? expr
[ : scalar-logical-expr ? expr ]... : expr )
"C1004 Each expr of a conditional-expr shall have the same declared type, kind
type parameters, and rank."
and
"Evaluation of a conditional-expr evaluates each scalar-logical-expr in
order, until the value of a scalar-logical-expr is true, or there are
no more scalar-logical-exprs. If the value of a scalar-logical-expr is
true, its subsequent expr is chosen; otherwise, the last expr of the
conditional-expr is chosen. The chosen expr is evaluated, and its
value is the value of the conditional expression."
I wonder whether we shouldn't just use 'expr' without true_/false_
prefix; I am especially stumbling over the underscore but also
"true/false expr" sounds a bit odd.
I'm open to different naming. In the GCC C frontend, the
build_conditional_expr function uses op1 and op2, while the Clang AST
uses an expression array with COND, LHS, and RHS
Note: I am not talking about the internal name - I am totally fine with
true_expr and false_expr.
I am talking about the wording presented to the Fortran user, who
will see the compiler error message.
Thus, "+ if (true_expr->rank != false_expr->rank)" is perfectly fine
with me.
But instead of:
Error: true_expr at (1) and false_expr at (2) in conditional expression must
have the same rank
I wonder whether it would be more Fortran like (without loosing
information) to say:
Error: expr at (1) and expr at (2) in conditional expression must have the same
rank
which avoids the underscore and true/false prefix.
* * *
BTW, just thinking about it: When simplifying, I wonder
whether we want to have a special case for
cond ? expr1 : expr1
Namely, condition is only known at runtime but both expressions are
identically? I think gfc_dep_compare_expr (...) == 0 does this.
(If you add it, please re-check that my claim is correct.)
* * *
I think for the beginning, that's okay – albeit a nicer 'sorry' would
be better and fully working .nil. best 😉
A nicer sorry still needs .nil. be parsed in the frontend. For now,
I'll keep it as a hard error and add the remaining functionality soon
🙂
Fine with me. Getting the first version in is more important. [Albeit
if you always print the sorry after parsing, it should be also simple;
the trickier part is to properly handle it - including printing an error
when it is invalid.]
* * *
I'm not sure if I'm understanding you correctly here. Are you
suggesting we treat the conditional expression as a special case for
EXPR_OP (the ternary operator)?
My initial thought, which isn't fully developed, is to store a nullptr
for the nil in the AST. However, this would require a null check every
time we use the true/false expr...
Not as ternery operator, only for .NIL.:
I was wondering about .NIL. as an operator that takes 0 expressions
with it - contrary to .NOT. that takes one (unary) and .AND. that takes
two (binary). - I got the idea because of the '.<something>.' it looks
very much like an operator and fits into the parser for operators.
But I have not thought whether that has additional implications or
how many code changes it requires.
Your idea of using nullptr is also interesting - and at a first glance,
should work. On the second thought, wouldn't this have issues (a) for
the diagnostic if gfc_expr == nullptr as there is also no expr->where
in that case. Additionally, (b) condition is known at compile time,
the might simplify to nullptr - which has issues. Having a special
case for EXPR_CONDITIONAL is fine - as it is localized, but adding
it elsewhere is a problem, especially as we already have several
null values in Fortran:
At least null(), c_null_ptr, and c_null_funptr come into my mind.
And '(cond ? c_null_ptr : .nil.)' looks like a valid expression,
depending on the associated dummy argument.
If you can get it working, having a nullptr for .NIL. works for me,
but I fear it won't work out and (too) many hacks are required.
But I am not sure whether there is a better third solution or whether
we can get one of the two ideas (.NIL. as EXPR_OP or as nullptr working).
* * *
Tobias
PS: I still have to read through the actual patch - instead of only replying
to your email ...
And I really need to look at your two sinpi library patches.
