Hi Tobias, On Wed, Sep 10, 2025 at 5:48 PM Tobias Burnus <[email protected]> wrote: > * * * > > Regarding the patch: > > > This patch adds support for conditional expressions in Fortran 2023 for > > limited > > types (logical, numerical), and also includes limited support for > > conditional > > arguments without `.nil.` support. > > 'limited types' sounds odd – in what sense is a type such as 'real' limited? > I think it should be either 'for some types' or 'for a limited set of types'. >
Fixed.
>
> > gcc/fortran/ChangeLog:
> >
> > * dump-parse-tree.cc (show_expr): Add support for EXPR_CONDITIONAL.
> > * expr.cc (gfc_get_conditional_expr): Add cond-expr constructor.
> > (gfc_copy_expr): Add support for EXPR_CONDITIONAL.
> > (free_expr0): Ditto.
> > (gfc_is_constant_expr): Ditto.
> > (simplify_conditional): Ditto.
> > (gfc_simplify_expr): Ditto.
> > (gfc_check_init_expr): Ditto.
> > (check_restricted): Ditto.
> > (gfc_traverse_expr): Ditto.
>
> Note that you can combine all function names in a single function, i.e.:
>
> * expr.cc (gfc_get_conditional_expr): Add cond-expr constructor.
> (gfc_copy_expr, free_expr0, gfc_is_constant_expr,
> simplify_conditional, gfc_simplify_expr, gfc_check_init_expr,
> check_restricted, gfc_traverse_expr): Add support for
> EXPR_CONDITIONAL.
>
> avoiding the long list of 'Ditto.'. You still need a ditto/likewise
> when you apply the same change to multiple files as in ...
>
> > * module.cc (mio_expr): Add support for EXPR_CONDITIONAL.
> > * resolve.cc (resolve_conditional): Ditto.
>
> (It also makes sense to use it, when you apply the same change as
> the previous entry to a function - but want to document some
> other change in addition.)
>
Fixed.
> * * *
>
> All in all, I think the patch is nearly ready to go.
> It handles most cases fine, there are still some corner
> case issues - and a wide swath of code unsupported but
> rejected with an error.
>
> My idea would be:
>
> (a) To add a sorry for arrays (i.e. expr->rank > 0) - those
> are a pretty common and will currently fail with an ICE.
>
> (b) Possibly, fix one or the other issue of the examples
> below [or all of them]
>
> (c) Defer the rest.
>
> [For (b), I think the se.pre/se.post issue should really be
> handled now, while the rest could be deferred.]
>
>
> Then, in follow-up work (one or multiple patches), address:
>
> (A) .NIL. as a feature
>
> (B) Handle the remaining non-array issues of below (if any)
>
> Once done, you can look at adding array, character,
> derived-type or polymophism support - or do a short detour
> to other topics and return.
>
> But now to the examples that illustrate some issues:
>
> * * *
>
> The following seems to require some additional check
> to yield a sorry:
>
> internal compiler error: in gfc_conv_variable, at fortran/trans-expr.cc:3197
>
> I wonder whether we should just disallow any array for now?
> I bet there are some special cases where it might work,
> but so far all array examples, I tried, fail.
>
>
> implicit none
> logical :: cond
> integer :: array(4)
> array=[1,2,3,4]
>
> cond = .true.
>
> print *, (cond ? array : array + f() ) ) ! ICE
> call sub( (cond ? array : array + f() ) ) ! ICE
> contains
> integer function f()
> f = 1
> error stop "should not be called"
> end
> subroutine sub(x)
> integer :: x(:)
> if (any (x /= [1,2,3,4])) error stop "invalid value"
> print *, x
> end
> end
>
Fixed.
> * * *
>
> Note: Even when rejecting arrays, gfortran might produce questionable code:
>
> implicit none
> integer, allocatable :: aa(:), bb(:)
> aa = [1,2]
>
> print *, (aa(1) > 0 ? aa(2) : f(bb(::2)))
> contains
> integer function f(x)
> integer :: x(*)
> f = 42
> end
> end
>
> However, it seems to work most of the time...
>
> * * *
>
> But, the following fails ('error stop'):
>
> implicit none
> integer :: aa(2)
> aa = [1,2]
>
> print *, (aa(1) > 0 ? aa(2) : g())
> contains
> integer function g()
> allocatable :: g
> error stop "should not be called"
> g = 3
> end
> end
>
> for the same reason.
>
>
> I think the solution for the two is:
>
> In gfc_conv_conditional_expr:
>
> if (true_se.pre || flase_se.pre)
> gfc_add_expr_to_block (&se->pre,
> fold_build3_loc (input_location, COND_EXPR, void_type_node,
> condition, true_se.pre ? gfc_finish_block (&true_se.pre) :
> build_empty_stmt (input_location)),
> (likewise for false_se)
>
> And also the same for {true,false}_se.post.
>
You're righ - the current implementation only uses COND_EXPR for the
value. But to properly handle side effects, we also need to handle the
pre/post blocks. Fixed now and added a test case: conditional_8.f90.
> * * *
>
> implicit none
> integer :: i,j
>
> do concurrent (i=j: 5) local(j) ! Error: Variable ‘j’ referenced in
> concurrent-header at (1) must not appear in LOCAL locality-spec
> end do
>
> do concurrent (i=(j > 1 ? j : 1) : 5) local(j) ! Not diagnosed
> end do
> end
>
Yep, that's a bug in this patch. We missed handling EXPR_CONDITIONAL
in gfc_expr_walker. Fixed it now and added a test case:
conditional_9.f90.
> * * *
>
> The following feels odd, i.e. for a compile-time condition,
> I'd expect a warning here - after all, 'gfc_resolve_expr' has
> been called - such that the condition should be simplifyable.
> (Again, compile with -fopenmp.)
>
> implicit none
> integer :: i
>
> !$omp simd simdlen( -1 ) ! Warning: INTEGER expression of SIMDLEN clause at
> (1) must be positive
> do i = 1, 4
> end do
>
> !$omp simd simdlen( (1 > 0 ? -1 : -2) ) ! No warning shown here
> do i = 1, 4
> end do
>
> end
>
> * * *
>
> The following is might be an issue in the caller, but nonetheless
> it fails with conditionals (gfc_match_omp_clauses [openmp.cc:2611]
> calling gfc_extract_int) - while it works with arithmetic expressions
> and named constants. (Compile with '-fopenmp'.)
>
> implicit none
> integer, parameter :: collapse_param = 1
> logical :: ll
> integer :: i
>
> ! !$omp do collapse( collapse_param ) ! OK
> ! !$omp do collapse( 2 - collapse_param ) OK
>
> !$omp do collapse( (1 > 0 ? 1 : 2) ) ! Fails with: 'Constant expression
> required'
> do i = 1, 4
> end do
> end
>
The two issues above can be resolved by adding the following code to
`gfc_match_dupl_check` in openmp.cc.
if (expr) gfc_simplify_expr (*expr, 0);
The `2 - x` should function correctly because, in addition to
gfc_simplify_expr, gfortran appears to perform basic folding in
matchexp.cc. However, for cond-expr, we depend on gfc_simplify_expr.
I am considering whether it should be committed first as a separate
patch, as it is not only related to cond-expr but a broader issue,
expr needs to be folded before verifying if it is a constant. I am
unsure if this is the most appropriate place to simplify.
Thanks,
Yuao
conditional.patch
Description: Binary data
