Improvements and bubbling up errors

Author: mzecc

src/example_fgen_basic/result/result_dp.f90

@@ -51,7 +51,7 @@ function constructor(data_v, error_v) result(self)
 
         type(ResultNone) :: build_res
 
-        build_res = self % build(data_v_in=data_v, error_v_in=error_v)
+        call self % build(data_v_in=data_v, error_v_in=error_v, res=build_res)
 
         if (build_res % is_error()) then
 
@@ -68,7 +68,7 @@ function constructor(data_v, error_v) result(self)
 
     end function constructor
 
-    function build(self, data_v_in, error_v_in) result(res)
+    subroutine build(self, data_v_in, error_v_in, res)
         !! Build instance
 
         class(ResultDP), intent(out) :: self
@@ -80,7 +80,7 @@ function build(self, data_v_in, error_v_in) result(res)
         class(ErrorV), intent(in), optional :: error_v_in
         !! Error message
 
-        type(ResultNone) :: res
+        type(ResultNone), intent(out) :: res
         !! Result
 
         if (present(data_v_in) .and. present(error_v_in)) then
@@ -99,7 +99,7 @@ function build(self, data_v_in, error_v_in) result(res)
 
         end if
 
-    end function build
+    end subroutine build
 
     subroutine finalise(self)
         !! Finalise the instance (i.e. free/deallocate)

src/example_fgen_basic/result/result_dp_manager.f90

@@ -1,25 +1,25 @@
-!> Manager of `ResultDP` (TODO: xref) across the Fortran-Python interface
+!> manager of `resultdp` (todo: xref) across the fortran-python interface
 module m_result_dp_manager
 
     use kind_parameters, only: dp
-    use m_error_v, only: ErrorV
-    use m_result_dp, only: ResultDP
-    use m_result_int, only: ResultInt
-    use m_result_none, only: ResultNone
+    use m_error_v, only: errorv
+    use m_result_dp, only: resultdp
+    use m_result_int, only: resultint
+    use m_result_none, only: resultnone
 
     implicit none(type, external)
     private
 
-    type(ResultDP), dimension(:), allocatable :: instance_array
+    type(resultdp), dimension(:), allocatable :: instance_array
     logical, dimension(:), allocatable :: instance_available
 
-    ! TODO: think about ordering here, alphabetical probably easiest
+    ! todo: think about ordering here, alphabetical probably easiest
     public :: build_instance, finalise_instance, get_available_instance_index, get_instance, set_instance_index_to, &
               ensure_instance_array_size_is_at_least
 
 contains
 
-    function build_instance(data_v_in, error_v_in) result(res_available_instance_index)
+    subroutine build_instance(data_v_in, error_v_in, res_available_instance_index)
         !! Build an instance
 
         real(kind=dp), intent(in), optional :: data_v_in
@@ -28,22 +28,28 @@ function build_instance(data_v_in, error_v_in) result(res_available_instance_ind
         class(ErrorV), intent(in), optional :: error_v_in
         !! Error message
 
-        type(ResultInt) :: res_available_instance_index
+        type(ResultInt) , intent(out) :: res_available_instance_index
         !! Index of the built instance
 
         type(ResultNone) :: res_build
 
         call ensure_instance_array_size_is_at_least(1)
         call get_available_instance_index(res_available_instance_index)
-        ! MZ check for errors ?
-        ! MZ function with side effect: good idea??
-        ! MZ why res_build is ResultNone??
-        res_build = instance_array(res_available_instance_index%data_v) % &
-                                  build(data_v_in=data_v_in, error_v_in=error_v_in)
 
-        ! TODO: check build has no error
+        if (res_available_instance_index % is_error()) return
 
-    end function build_instance
+        call instance_array(res_available_instance_index%data_v) % &
+                      build(data_v_in=data_v_in, error_v_in=error_v_in, res=res_build)
+
+        ! Check if build failed
+        if (res_build % is_error()) then
+          ! free slot again
+          instance_available(res_available_instance_index%data_v) = .true.
+          ! bubble the error up as ResultInt
+          res_available_instance_index = ResultInt(error_v=res_build%error_v)
+        end if
+
+    end subroutine build_instance
 
     subroutine finalise_instance(instance_index)
         !! Finalise an instance
@@ -89,8 +95,9 @@ subroutine get_available_instance_index(res_available_instance_index)
         end do
 
         ! TODO: switch to returning a Result type with an error set
-        res_available_instance_index = ResultInt(error_v=ErrorV(code=1, message="No available instances"))
         ! error stop 1
+        res_available_instance_index = ResultInt(error_v=ErrorV(code=1, message="No available instances"))
+
     end subroutine get_available_instance_index
 
     ! Change to pure function when we update check_index_claimed to be pure
@@ -135,6 +142,8 @@ function check_index_claimed(instance_index) result(res_check_index_claimed)
         !! Instance index to check
         type(ResultNone) :: res_check_index_claimed
         character(len=:), allocatable :: msg
+        ! msg initialisation to avoid compiler warning
+        msg = ""
 
         if (instance_available(instance_index)) then
             ! TODO: Switch to using Result here
@@ -146,8 +155,6 @@ function check_index_claimed(instance_index) result(res_check_index_claimed)
             ! res = ResultNone(ResultDP(code=1, message="Index ", instance_index, " has not been claimed"))
             ! print *, "Index ", instance_index, " has not been claimed"
             ! error stop 1
-            ! MZ Weird thing allocatable message
-            msg = ""
             write(msg,fmt="(A, I0, A)") "Index ", instance_index," has not been claimed"
             res_check_index_claimed = ResultNone(error_v=ErrorV(code=1, message=msg))
         end if

src/example_fgen_basic/result/result_dp_wrapper.f90

@@ -1,9 +1,10 @@
 !> Wrapper for interfacing `m_result_dp` with Python
 module m_result_dp_w
 
-    use m_error_v, only: ErrorV
+    use m_error_v, only: ErrorV, NO_ERROR_CODE
     use m_result_dp, only: ResultDP
     use m_result_int, only: ResultInt
+    use m_result_none, only: ResultNone
 
     ! The manager module, which makes this all work
     use m_error_v_manager, only: &
@@ -25,22 +26,21 @@ module m_result_dp_w
               ensure_at_least_n_instances_can_be_passed_simultaneously, &
               data_v_is_set, get_data_v, error_v_is_set, get_error_v
 
+   ! Annoying that this has to be injected everywhere,
+   ! but ok it can be automated.
+   integer, parameter :: dp = selected_real_kind(15, 307)
+
 contains
 
     subroutine build_instance(data_v, error_v_instance_index, res_available_instance_index)
         !! Build an instance
 
-        ! Annoying that this has to be injected everywhere,
-        ! but ok it can be automated.
-        integer, parameter :: dp = selected_real_kind(15, 307)
-
         real(kind=dp), intent(in), optional :: data_v
         !! Data
 
         integer, intent(in), optional :: error_v_instance_index
         !! Error
 
-        ! integer, intent(out) :: instance_index
         type(ResultInt), intent(out) :: res_available_instance_index
         !! Instance index of the built instance
         !
@@ -51,9 +51,15 @@ subroutine build_instance(data_v, error_v_instance_index, res_available_instance
         ! We use the manager layer to initialise the attributes before passing on.
         type(ErrorV) :: error_v
 
-        error_v = error_v_manager_get_instance(error_v_instance_index)
+        if (present(error_v_instance_index)) then
+            error_v = error_v_manager_get_instance(error_v_instance_index)
+        else
+            ! No error provided: initialize empty error
+            error_v%code = NO_ERROR_CODE
+            error_v%message = ""
+        end if
 
-        res_available_instance_index = result_dp_manager_build_instance(data_v, error_v)
+        call result_dp_manager_build_instance(data_v, error_v, res_available_instance_index)
 
     end subroutine build_instance
 
@@ -130,10 +136,6 @@ subroutine get_data_v( &
         data_v &
         )
 
-        ! Annoying that this has to be injected everywhere,
-        ! but ok it can be automated.
-        integer, parameter :: dp = selected_real_kind(15, 307)
-
         integer, intent(in) :: instance_index
 
         real(kind=dp), intent(out) :: data_v

src/example_fgen_basic/result/result_int.f90

@@ -51,7 +51,7 @@ function constructor(data_v, error_v) result(self)
 
         type(ResultNone) :: build_res
 
-        build_res = self % build(data_v_in=data_v, error_v_in=error_v)
+        call self % build(data_v_in=data_v, error_v_in=error_v, res= build_res)
 
         if (build_res % is_error()) then
 
@@ -68,19 +68,19 @@ function constructor(data_v, error_v) result(self)
 
     end function constructor
 
-    function build(self, data_v_in, error_v_in) result(res)
+    subroutine build(self, data_v_in, error_v_in, res)
         !! Build instance
 
-        class(ResultInt), intent(out) :: self
-        ! Hopefully can leave without docstring (like Python)
-
         integer(kind=i8), intent(in), optional :: data_v_in
         !! Data
 
         class(ErrorV), intent(in), optional :: error_v_in
         !! Error message
 
-        type(ResultNone) :: res
+        class(ResultInt), intent(inout) :: self
+        ! Hopefully can leave without docstring (like Python)
+
+        type(ResultNone), intent(inout) :: res
         !! Result
 
         if (present(data_v_in) .and. present(error_v_in)) then
@@ -99,7 +99,7 @@ function build(self, data_v_in, error_v_in) result(res)
 
         end if
 
-    end function build
+    end subroutine build
 
     subroutine finalise(self)
         !! Finalise the instance (i.e. free/deallocate)

src/example_fgen_basic/result/result_int.py

@@ -22,7 +22,7 @@
 @define
 class ResultInt:
     """
-    Result type that can hold double precision real values
+    Result type that can hold (8 bit) integer values
     """
 
     # TODO: add validation that one of data_v and error_v is provided but not both

src/example_fgen_basic/result/result_int_manager.f90

@@ -34,8 +34,7 @@ function build_instance(data_v_in, error_v_in) result(instance_index)
 
         call ensure_instance_array_size_is_at_least(1)
         call get_available_instance_index(instance_index)
-        res_build = instance_array(instance_index) % build(data_v_in=data_v_in, error_v_in=error_v_in)
-        ! MZ: Is the line above correct??
+        call instance_array(instance_index) % build(data_v_in=data_v_in, error_v_in=error_v_in, res=res_build)
         ! TODO: check build has no error
 
     end function build_instance

src/example_fgen_basic/result/result_int_wrapper.f90

@@ -16,23 +16,23 @@ module m_result_int_w
         result_int_manager_finalise_instance => finalise_instance, &
         result_int_manager_get_instance => get_instance, &
         result_int_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least
-        !MZ: Set instance?
+
     implicit none(type, external)
     private
 
     public :: build_instance, finalise_instance, finalise_instances, &
               ensure_at_least_n_instances_can_be_passed_simultaneously, &
               data_v_is_set, get_data_v, error_v_is_set, get_error_v
 
+    ! Annoying that this has to be injected everywhere,
+    ! but ok it can be automated.
+    integer, parameter :: i8 = selected_int_kind(18)
+
 contains
 
     subroutine build_instance(data_v, error_v_instance_index, instance_index)
         !! Build an instance
 
-        ! Annoying that this has to be injected everywhere,
-        ! but ok it can be automated.
-        integer, parameter :: i8 = selected_int_kind(18)
-
         integer(kind=i8), intent(in), optional :: data_v
         !! Data
 
@@ -128,10 +128,6 @@ subroutine get_data_v( &
         data_v &
         )
 
-        ! Annoying that this has to be injected everywhere,
-        ! but ok it can be automated.
-        integer, parameter :: i8 = selected_int_kind(18)
-
         integer, intent(in) :: instance_index
 
         integer(kind=i8), intent(out) :: data_v