Skip to content

error_stop to stderr and optional returncode #53

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

Merged
merged 4 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ enable_testing()
# this avoids stdlib and projects using stdlib from having to introspect stdlib's directory structure
set(CMAKE_Fortran_MODULE_DIRECTORY ${CMAKE_BINARY_DIR})

# compiler feature checks
include(CheckFortranSourceCompiles)
check_fortran_source_compiles("error stop i; end" f18errorstop SRC_EXT f90)

add_subdirectory(src)
6 changes: 6 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ set(SRC

add_library(fortran_stdlib ${SRC})

if(f18errorstop)
target_sources(fortran_stdlib PRIVATE f18estop.f90)
else()
target_sources(fortran_stdlib PRIVATE f08estop.f90)
endif()

add_subdirectory(tests)

install(TARGETS fortran_stdlib
Expand Down
39 changes: 39 additions & 0 deletions src/f08estop.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
submodule (stdlib_experimental_error) estop

contains

module procedure error_stop
! Aborts the program with nonzero exit code
! this is a fallback for Fortran 2008 error stop (e.g. Intel 19.1/2020 compiler)
!
! The "stop <character>" statement generally has return code 0.
! To allow non-zero return code termination with character message,
! error_stop() uses the statement "error stop", which by default
! has exit code 1 and prints the message to stderr.
! An optional integer return code "code" may be specified.
!
! Example
! -------
!
! call error_stop("Invalid argument")

write(stderr,*) msg

if(present(code)) then
select case (code)
case (1)
error stop 1
case (2)
error stop 2
case (77)
error stop 77
case default
write(stderr,*) 'ERROR: code ',code,' was specified.'
error stop
end select
else
error stop
endif
end procedure

end submodule
27 changes: 27 additions & 0 deletions src/f18estop.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
submodule (stdlib_experimental_error) estop

contains

module procedure error_stop
! Aborts the program with nonzero exit code
!
! The "stop <character>" statement generally has return code 0.
! To allow non-zero return code termination with character message,
! error_stop() uses the statement "error stop", which by default
! has exit code 1 and prints the message to stderr.
! An optional integer return code "code" may be specified.
!
! Example
! -------
!
! call error_stop("Invalid argument")

if(present(code)) then
write(stderr,*) msg
error stop code
else
error stop msg
endif
end procedure

end submodule estop
31 changes: 12 additions & 19 deletions src/stdlib_experimental_error.f90
Original file line number Diff line number Diff line change
@@ -1,41 +1,34 @@
module stdlib_experimental_error
use, intrinsic :: iso_fortran_env, only: stderr=>error_unit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not need to be imported here, does it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this patch and it works:

diff --git a/src/f08estop.f90 b/src/f08estop.f90
index d501978..17ea448 100644
--- a/src/f08estop.f90
+++ b/src/f08estop.f90
@@ -1,4 +1,5 @@
 submodule (stdlib_experimental_error) estop
+use, intrinsic :: iso_fortran_env, only: stderr=>error_unit
 
 contains
 
diff --git a/src/f18estop.f90 b/src/f18estop.f90
index ea83de7..6be7900 100644
--- a/src/f18estop.f90
+++ b/src/f18estop.f90
@@ -1,4 +1,5 @@
 submodule (stdlib_experimental_error) estop
+use, intrinsic :: iso_fortran_env, only: stderr=>error_unit
 
 contains
 
diff --git a/src/stdlib_experimental_error.f90 b/src/stdlib_experimental_error.f90
index 3d932d6..135722b 100644
--- a/src/stdlib_experimental_error.f90
+++ b/src/stdlib_experimental_error.f90
@@ -1,5 +1,4 @@
 module stdlib_experimental_error
-use, intrinsic :: iso_fortran_env, only: stderr=>error_unit
 implicit none
 private

Copy link
Member Author

@scivision scivision Jan 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's more of a deduplication issue. The scope of submodule is such that these submodules pickup stderr from the parent module stdlib_experimental_error e.g. imagine there are dozens of submodules (which eventually there will be in this project) they don't all need to each import the same modules, just import once in the parent or ancestor module.
i.e. part of the "point" of using submodules is deduplication, and that can also include deduplicating use statements that many submodules use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that's fine. I don't use submodules myself, so I don't know what the best way to use them is. I thought the idea is that the main module is sort of like a C++ .h file and the submodule is then like .cpp file. In C++, the accepted practice is to only include things in the .h files that are needed to specify the API. Everything else is included in the .cpp files. In the same spirit, since stderr is not needed for the API, I thought it would make sense to only include it in the submodule.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could use module/submodule that way, but they are generally not used that way as the scoping rules are different than C++. I use submodules for almost every program for reasons including:

  • very fast rebuild if procedure API didn't change, can be 100x or more build wallclock speed, important for big programs that take several minutes to build fully, but only a couple seconds if rebuilding after submodule change
  • allows switching in/out capabilities or libraries. Examples
    • file IO with HDF5, NetCDF4 or fallback to raw binary if they're not available, switched by the build systems auto/manual
    • external procedures/modules, maybe some are proprietary or take a long build time or a lot of prereqs, make them optional yet almost instantly switchable in/out via build system
    • procedure switching, maybe the procedures are overloaded in certain ways as defined by a simulation, again making almost instant rebuilds on big programs with build system options

So submodule can be used like one step up from "just in time" compilation, I call it "build on run" since based on options say called from Python API, I can rebuild huge Fortran program based on options user specifies from Python (or directly in CMake) in seconds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

submodule can also be used for build-time polymorphism, avoiding need for preprocessing and again shrinking memory use and build time by only building the required kinds.
I believe a significant part of the preprocessing and generation seen in Fortran could be replaced more cleanly and readably with submodule, select type and select rank. Of course will still be good applications for a small #ifdef foo #endif and generation.

implicit none
private

interface ! f{08,18}estop.f90
module subroutine error_stop(msg, code)
character(*), intent(in) :: msg
integer, intent(in), optional :: code
end subroutine error_stop
end interface

public :: assert, error_stop

contains

subroutine assert(condition)
subroutine assert(condition, code)
! If condition == .false., it aborts the program.
!
! Arguments
! ---------
!
logical, intent(in) :: condition
integer, intent(in), optional :: code
!
! Example
! -------
!
! call assert(a == 5)

if (.not. condition) call error_stop("Assert failed.")
end subroutine

subroutine error_stop(msg)
! Aborts the program with nonzero exit code
!
! The statement "stop msg" will return 0 exit code when compiled using
! gfortran. error_stop() uses the statement "stop 1" which returns an exit code
! 1 and a print statement to print the message.
!
! Example
! -------
!
! call error_stop("Invalid argument")

character(len=*) :: msg ! Message to print on stdout
print *, msg
stop 1
if (.not. condition) call error_stop("Assert failed.", code)
end subroutine

end module
9 changes: 9 additions & 0 deletions src/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
add_subdirectory(ascii)
add_subdirectory(loadtxt)

add_executable(test_skip test_skip.f90)
target_link_libraries(test_skip fortran_stdlib)
add_test(NAME AlwaysSkip COMMAND $<TARGET_FILE:test_skip>)
set_tests_properties(AlwaysSkip PROPERTIES SKIP_RETURN_CODE 77)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this test? What is it testing? When I run ctest, it outputs:

$ ctest 
Test project /home/ondrej/repos/stdlib
    Start 2: AlwaysFail
    Start 3: ASCII
    Start 4: load_text
    Start 5: save_text
1/5 Test #2: AlwaysFail .......................   Passed    0.01 sec
2/5 Test #3: ASCII ............................   Passed    0.01 sec
3/5 Test #4: load_text ........................   Passed    0.01 sec
4/5 Test #5: save_text ........................   Passed    0.01 sec
    Start 1: AlwaysSkip
5/5 Test #1: AlwaysSkip .......................***Skipped   0.01 sec

100% tests passed, 0 tests failed out of 5

Total Test time (real) =   0.02 sec

The following tests did not run:
	  1 - AlwaysSkip (Skipped)

Which seems confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's testing that I can pass arbitrary return codes. There's a de facto standard that 77 means to skip a test, so 77 was a convenient alternative to the oft-used returncode 1 that was tested in AlwaysFail


add_executable(test_fail test_fail.f90)
target_link_libraries(test_fail fortran_stdlib)
add_test(NAME AlwaysFail COMMAND $<TARGET_FILE:test_fail>)
set_tests_properties(AlwaysFail PROPERTIES WILL_FAIL true)
4 changes: 2 additions & 2 deletions src/tests/loadtxt/test_loadtxt.f90
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
program test_loadtxt
use iso_fortran_env, only: sp=>real32, dp=>real64 ,qp=>real128
use stdlib_experimental_io, only: loadtxt
use stdlib_experimental_error, only: error_stop
implicit none

real(sp), allocatable :: s(:, :)
Expand Down Expand Up @@ -46,8 +47,7 @@ subroutine print_array(a)
print *, a(i, :)
end do
class default
write(*,'(a)')'The proposed type is not supported'
error stop
call error_stop('The proposed type is not supported')
end select

end subroutine
Expand Down
8 changes: 8 additions & 0 deletions src/tests/test_fail.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
program AlwaysFail

use stdlib_experimental_error, only : assert
implicit none

call assert(.false.)

end program
8 changes: 8 additions & 0 deletions src/tests/test_skip.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
program AlwaysSkip

use stdlib_experimental_error, only : assert
implicit none

call assert(.false., 77)

end program