Skip to content

[Feedback] Why not use merge but create another optval? #448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zoziha opened this issue Jun 28, 2021 · 7 comments
Closed

[Feedback] Why not use merge but create another optval? #448

zoziha opened this issue Jun 28, 2021 · 7 comments
Labels
idea Proposition of an idea and opening an issue to discuss it

Comments

@zoziha
Copy link
Contributor

zoziha commented Jun 28, 2021

Description

merge is a standard function of Fortran that has been implemented, but stdlib has created a generic function optval (optval has a more fixed scope). I think the two functions overlap, and I think it may be stdlib's obligation to use the standard function.
(See 202X feature: Conditional Expressions)
Prior Art

program main
    
    call check_merge(1.)
    call check_merge()

contains

    subroutine check_merge(opt)
        real, optional :: opt
        
        print *, merge(opt, 2., present(opt))
        !! print *, optval(opt, 2.)

    end subroutine check_merge

end program main
@zoziha zoziha added the idea Proposition of an idea and opening an issue to discuss it label Jun 28, 2021
@awvwgk
Copy link
Member

awvwgk commented Jun 28, 2021

This fails with at least one compiler (nvfortran) with a segmentation fault as merge is not required to short-circuit the evaluation.

@Romendakil
Copy link

Does stdlib actually compile and run with nvfortran? My own experience in our projects is that nvfortran does compile a lot of code, so it understands the syntax, but many implementations are quite buggy, and I would even hesitate to call it a complete F2003 compiler.

@awvwgk
Copy link
Member

awvwgk commented Jun 28, 2021

I have tried compiling it a few times because I'm using nvfortran for one of my projects, but so far without success (main issue is missing quadruple precision support). I totally agree that nvfortran has a very unstable front-end, which makes it a real challenge to work with. Same holds for the current flang compiler which uses basically the same front-end.

There is hope that nvfortran will at some point use the f18 front-end instead of the current pgfortran based one, but this is still a long way to go.

Related issue: #107

@zoziha
Copy link
Contributor Author

zoziha commented Jul 1, 2021

In the situation I encountered today, merge(character(6) :: format, character(4) :: "(g0)", present(character(6) :: format)) will report an error: "Unequal character lengths ( 6/4) in MERGE intrinsic".
But the optval routine will not report such an error.
This is because the function definition of merge is stricter. RESULT = MERGE (TSOURCE, FSOURCE, MASK) requires: FSOURCE shall be of the same type and type parameters as TSOURCE. (See https://gcc.gnu.org/onlinedocs/gfortran/MERGE.html)

This round, optval wins in character variables involving character length!

@milancurcic
Copy link
Member

On a related topic, conditional expression syntax was accepted today for F202X (hopefully F2023). You will then be able to do

! print *, merge(opt, 2., present(opt)) ! invalid

print *, (present(opt) ? opt : 2.) ! F202X

Syntax paper here.

@zoziha
Copy link
Contributor Author

zoziha commented Jul 13, 2021

At this stage, we can use both the merge and optval functions in stdlib, only when the object is a string of different lengths, we have to use optval function.
Because merge is a standard function, we can reduce optval module dependencies by using it(merge).
It is convenient to replace them with the conditional expression syntax (F202X) in the future.

@zoziha zoziha closed this as completed Jul 24, 2021
@zoziha
Copy link
Contributor Author

zoziha commented Aug 5, 2021

(From https://github.com/fortran-lang/stdlib/pull/480/files#r682911486)

Using present in merge be standard non-conforming and possibly segfault.

The use of merge(t, f, present(t)) indeed occur errors in ifx/llvm-flang compilers, not ifort/gfortran compilers.

image
So, we should use optval in this case.😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Proposition of an idea and opening an issue to discuss it
Projects
None yet
Development

No branches or pull requests

4 participants