Hi Yuao,
Yuao Ma wrote:
The updated patch will resolve the first two cases. Regarding the
coarray, if it's not causing an ICE, I'd prefer to postpone working on
it until we handle arrays.
Makes sense. Coarrays also behave strangely at times and
are not that widely used. Still, I want to mention it as
they sometimes behave like normal variables and sometimes
not.
* * *
Side remark: I was wondering about the diagnostic for:
subroutine sub(x)
character(5), optional :: x
and passing a too-short string.
Result: No message for:
call sub( (cc ? "132" : "12"))
but for
call sub( (cc ? "132" : "345"))
we get:
Warning: Character length of actual argument shorter
than of dummy argument ‘x’ (3/5) at (1)
Thus, this part seems to work FINE 🙂
(It is possible to warn for the first case, but it is
rather special, hence, I don't expect a warning for it
by adding a specific test for it.)
Currently, our string_length underneath is also a COND_EXPR. This
warning will only be triggered if the string_length is a constant (or
simplified to a constant). In the second example, the string_length of
both operands is the same, so we will receive the warning. However, in
the first example, we will not.
Yes, hence, I meant that it works correctly.
* * *
This is a great test case because it revealed a subtle bug in the
patch: when retrieving the optional pointer, we should use
null_pointer_node instead of NOP_EXPR from build_empty_statement.
Using the latter introduces subtle bugs into the CFG - I suspect some
missing GOTOs - which ultimately leads to errors.
Ups. Good that this is now fixed.
From 8ee6b564fca20862b9da55a5fe1e8f90f82bd317 Mon Sep 17 00:00:00 2001
From: Yuao Ma<[email protected]>
Date: Sun, 26 Oct 2025 14:50:28 +0800
Subject: [PATCH] fortran: support .NIL. in conditional arguments
TBD
I am not sure whether some longer description makes sense or if we just
want to go with the subject line and the by-file changelog – but in any
case 'TBD' looks wrong.
* * *
+++ b/gcc/fortran/trans-expr.cc
@@ -4375,6 +4375,17 @@ gfc_conv_conditional_expr (gfc_se *se, gfc_expr *expr)
tree condition, true_val, false_val;
tree type;
+ /* Handle .NIL. */
+ if (!expr->value.conditional.condition)
+ {
+ if (se->want_pointer || expr->ts.type == BT_CHARACTER)
+ se->expr = null_pointer_node;
+ else
+ se->expr = integer_zero_node;
+ se->string_length = build_int_cst (gfc_charlen_type_node, 0);
+ return;
+ }
This code looks wrong.
And, indeed, I get:
Error: non-trivial conversion in ‘integer_cst’
integer(kind=16)
integer(kind=4)
D.4685 = 0;
That's for the following:
----------------
module m
implicit none (type, external)
contains
subroutine sub(x)
integer(16), value,optional :: x
end subroutine
end module m
use m
logical :: c1
c1 = .true.
call sub((c1 ? .nil. : 1_16))
call sub((.not. c1 ? .nil. : 1_16))
call sub((c1 ? 1_16 : .nil.))
end
----------------
// Note: I test both false and true branch - too easy to screw up here ...
// and I like to do so. Likewise for nested conditions.
How about the following? - Note the 'build_zero_cst',
which handles also non-integers (and honors the type of
integer).
======================================
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -4375,2 +4375,2 @@ gfc_conv_conditional_expr (gfc_se *se, gfc_expr *expr)
- tree condition, true_val, false_val;
- tree type;
+ tree condition, true_val = NULL_TREE, false_val;
+ tree type = NULL_TREE;
@@ -4386,6 +4386,34 @@ gfc_conv_conditional_expr (gfc_se *se, gfc_expr *expr)
- true_se.want_pointer = se->want_pointer;
- gfc_conv_expr (&true_se, expr->value.conditional.true_expr);
- true_val = true_se.expr;
- false_se.want_pointer = se->want_pointer;
- gfc_conv_expr (&false_se, expr->value.conditional.false_expr);
- false_val = false_se.expr;
+ /* Handle .NIL.; note that either true_expr or false_expr is not .NIL. */
+ if (expr->value.conditional.true_expr->expr_type == EXPR_CONDITIONAL
+ && !expr->value.conditional.true_expr->value.conditional.condition)
+ ; /* defer to after false_expr. */
+ else
+ {
+ true_se.want_pointer = se->want_pointer;
+ gfc_conv_expr (&true_se, expr->value.conditional.true_expr);
+ true_val = true_se.expr;
+ type = TREE_TYPE (true_val);
+ }
+
+ if (expr->value.conditional.false_expr->expr_type == EXPR_CONDITIONAL
+ && !expr->value.conditional.false_expr->value.conditional.condition)
+ {
+ false_val = build_zero_cst (type);
+ if (expr->ts.type == BT_CHARACTER)
+ false_se.string_length = build_int_cst (gfc_charlen_type_node, 0);
+ }
+ else
+ {
+ false_se.want_pointer = se->want_pointer;
+ gfc_conv_expr (&false_se, expr->value.conditional.false_expr);
+ false_val = false_se.expr;
+ }
+
+ if (expr->value.conditional.true_expr->expr_type == EXPR_CONDITIONAL
+ && !expr->value.conditional.true_expr->value.conditional.condition)
+ {
+ type = TREE_TYPE (false_val);
+ true_val = build_zero_cst (type);
+ if (expr->ts.type == BT_CHARACTER)
+ true_se.string_length = build_int_cst (gfc_charlen_type_node, 0);
+ }
@@ -4415,4 +4442,0 @@ gfc_conv_conditional_expr (gfc_se *se, gfc_expr *expr)
- type = gfc_typenode_for_spec (&expr->ts);
- if (se->want_pointer)
- type = build_pointer_type (type);
-
======================================
Actually, I probably should not have deleted the last item – and
it probably should be moved up before the gfc_conv_expr code block;
The reason is that for
(cond ? "ab" : "abcd")
one is size 16 bit, the other size 32 bit, which is visible
in the type.
Presumably, it does not matter most of the time, but I would
not rule out that it might in some special case.
* * *
@@ -6688,16 +6699,16 @@ conv_dummy_value (gfc_se * parmse, gfc_expr * e,
gfc_symbol * fsym,
{
/* F2018:15.5.2.12 Argument presence and
restrictions on arguments not present. */
- if (e->expr_type == EXPR_VARIABLE
- && e->rank == 0
- && (gfc_expr_attr (e).allocatable
- || gfc_expr_attr (e).pointer))
+ if ((e->expr_type == EXPR_VARIABLE && e->rank == 0
+ && (gfc_expr_attr (e).allocatable || gfc_expr_attr (e).pointer))
+ || e->expr_type == EXPR_CONDITIONAL)
{
The existing code is to handle:
subroutine sub(x)
integer, optional :: x
end
allocatable(y)
call sub(y) ! y is not allocated → regard as absent argument
If you look at -fdump-tree-original, your current patch produces
for '(c1 ? .nil. : 1)' odd code like:
D.1 = !c1
D.2 = !c1
if (D.1)
(void) 0
else
D.3 = 1;
sub(D.1 ? 0 : D.2 ? 0 : 1, ...)
I think it makes more sense to handle conditionals differently.
How about using instead:
===============================
@@ -6709,0 +6733,6 @@ conv_dummy_value (gfc_se * parmse, gfc_expr * e,
gfc_symbol * fsym,
+ else if (e->expr_type == EXPR_CONDITIONAL)
+ {
+ gcc_assert (parmse && TREE_CODE (parmse->expr) == COND_EXPR);
+ tree cond = TREE_OPERAND (parmse->expr, 0);
+ vec_safe_push (optionalargs, fold_convert (boolean_type_node, cond));
+ }
===============================
* * *
Side remark: I should definitely use more testcases with
real, character, complex, ... and not just integer.
* * *
+++ b/gcc/testsuite/gfortran.dg/conditional_10.f90
@@ -0,0 +1,28 @@
+! { dg-do run }
...
+ call sub((c ? "123" : .NIL.))
+contains
...
+ subroutine sub(x)
+ character(*), optional :: x
+ end subroutine sub
+end program conditional_nil
I think it would be good to have both a present
and absent/.nil. check here. I think you could also
check for the string LEN and the value here.
* * *
+++ b/gcc/testsuite/gfortran.dg/conditional_12.f90
@@ -0,0 +1,33 @@
+! { dg-do run }
...
+ subroutine sub_str(l, x)
+ integer, value :: l
+ character, value, optional :: x
+ if (present(x)) then
+ if (l /= LEN(x)) error stop
Can we have a test which checks that the present/absent
is correctly handled? Currently, it only checks at runtime
the present case. The inner test is not really effective as
'len(x)' is statically optimized to '1'.
(BTW: Supporting len > 1 and arrays with VALUE is a
Fortran 2018 feature that gfortran does not yet handle.)
Hence, ...
+ cc = .true.
+ call sub_str(1, (cc ? "abc" : .nil.))
+ end
I think it make sense to have a .nil. case here and
pass the status (absent/present) as additional arg.
Thanks for your patch!
Tobias
