From 2f4e9c0f4fd9fb73b9a30ff46c602fc3cdb6de2d Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Fri, 12 Jun 2026 15:46:47 +0200 Subject: [PATCH 1/3] fix: pass NULL for absent optional arguments in --direct-c The direct-C wrapper kept a non-NULL pointer for an optional scalar set to None (it only zeroed the value) and allocated a blank buffer for an optional character input. Both made present(arg) return .true. inside Fortran, so default-value branches never ran. Issue #369. Set the pointer to NULL in both cases, matching the f2py path. Optional output characters still allocate their buffer. Add examples/issue369_optional_present, covering optional scalar, character, and array arguments through both the f2py and direct-C paths. --- examples/Makefile | 1 + examples/issue369_optional_present/Makefile | 33 +++++++++++++ .../issue369_optional_present/Makefile.meson | 6 +++ examples/issue369_optional_present/main.f90 | 47 +++++++++++++++++++ examples/issue369_optional_present/tests.py | 39 +++++++++++++++ f90wrap/directc_cgen/arguments_scalar.py | 8 +++- 6 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 examples/issue369_optional_present/Makefile create mode 100644 examples/issue369_optional_present/Makefile.meson create mode 100644 examples/issue369_optional_present/main.f90 create mode 100644 examples/issue369_optional_present/tests.py diff --git a/examples/Makefile b/examples/Makefile index 1f116c8bb..f8be55856 100644 --- a/examples/Makefile +++ b/examples/Makefile @@ -41,6 +41,7 @@ EXAMPLES = \ issue32 \ issue353_selected_kind \ issue357_name_conflict \ + issue369_optional_present \ issue41_abstract_classes \ keep_single_interface \ keyword_renaming_issue160 \ diff --git a/examples/issue369_optional_present/Makefile b/examples/issue369_optional_present/Makefile new file mode 100644 index 000000000..88ce63ea5 --- /dev/null +++ b/examples/issue369_optional_present/Makefile @@ -0,0 +1,33 @@ +#======================================================================= +# define the compiler names +#======================================================================= + +include ../make.inc +PY_MOD = pywrapper +F90_SRC = main.f90 +OBJ = $(F90_SRC:.f90=.o) +F90WRAP_SRC = $(addprefix f90wrap_,${F90_SRC}) +WRAPFLAGS = -v +F2PY = f2py-f90wrap +SRC_AR = libsrc.a +.PHONY: all clean + +all: test + +clean: + rm -rf *.mod *.smod *.o ${SRC_AR} f90wrap*.f90 ${PY_MOD}.py _${PY_MOD}*.so __pycache__/ .f2py_f2cmap build ${PY_MOD}/ + +%.o: %.f90 + ${F90} ${F90FLAGS} -c $< -o $@ + +${F90WRAP_SRC}: ${OBJ} + ${F90WRAP} -m ${PY_MOD} ${WRAPFLAGS} ${F90_SRC} + +${SRC_AR}: ${OBJ} + ar rcs $@ ${OBJ} + +f2py: ${F90WRAP_SRC} ${SRC_AR} + CFLAGS="${CFLAGS}" ${F2PY} -c -m _${PY_MOD} ${F2PYFLAGS} f90wrap_*.f90 -L. -lsrc + +test: f2py + ${PYTHON} tests.py diff --git a/examples/issue369_optional_present/Makefile.meson b/examples/issue369_optional_present/Makefile.meson new file mode 100644 index 000000000..b2ee9928b --- /dev/null +++ b/examples/issue369_optional_present/Makefile.meson @@ -0,0 +1,6 @@ +include ../make.meson.inc + +NAME := pywrapper + +test: build + $(PYTHON) tests.py diff --git a/examples/issue369_optional_present/main.f90 b/examples/issue369_optional_present/main.f90 new file mode 100644 index 000000000..5588a794c --- /dev/null +++ b/examples/issue369_optional_present/main.f90 @@ -0,0 +1,47 @@ +! Regression example for issue #369. +! +! With --direct-c, passing None for an OPTIONAL argument must make present(arg) +! return .false. inside Fortran. The bug passed a non-NULL pointer (scalar) or an +! allocated blank buffer (character), so present() wrongly returned .true. +! +! Each subroutine reports its decision through an intent(out) result so the Python +! test can assert which branch ran. Both the regular f90wrap+f2py path and the +! --direct-c path are exercised by the shared tests.py. +module m_optional + implicit none +contains + + ! Optional scalar: the exact case from issue #369. + subroutine scalar_default(x, n) + real, intent(out) :: x + integer, intent(in), optional :: n + if (.not. present(n)) then + x = 42.0 + else + x = real(n) + end if + end subroutine scalar_default + + ! Optional character input. + subroutine char_default(label, name) + character(len=16), intent(out) :: label + character(len=*), intent(in), optional :: name + if (present(name)) then + label = name + else + label = "ABSENT" + end if + end subroutine char_default + + ! Optional array input (already correct before the fix: a regression lock). + subroutine array_default(s, v) + integer, intent(out) :: s + real, intent(in), optional :: v(:) + if (present(v)) then + s = size(v) + else + s = -1 + end if + end subroutine array_default + +end module m_optional diff --git a/examples/issue369_optional_present/tests.py b/examples/issue369_optional_present/tests.py new file mode 100644 index 000000000..b5de406db --- /dev/null +++ b/examples/issue369_optional_present/tests.py @@ -0,0 +1,39 @@ +import unittest + +import numpy as np + +from pywrapper import m_optional + + +def as_text(value): + """Character returns come back as bytes (direct-c) or str (f2py).""" + if isinstance(value, bytes): + value = value.decode() + return value.strip() + + +class TestOptionalPresent(unittest.TestCase): + def test_scalar_present(self): + self.assertEqual(m_optional.scalar_default(n=5), 5.0) + + def test_scalar_absent_uses_default(self): + # Bug #369: direct-c saw present(n) == .true. with value 0. + self.assertEqual(m_optional.scalar_default(), 42.0) + + def test_char_present(self): + self.assertEqual(as_text(m_optional.char_default(name="hello")), "hello") + + def test_char_absent_uses_default(self): + self.assertEqual(as_text(m_optional.char_default()), "ABSENT") + + def test_array_present(self): + self.assertEqual( + m_optional.array_default(v=np.array([1.0, 2.0, 3.0], dtype=np.float32)), 3 + ) + + def test_array_absent(self): + self.assertEqual(m_optional.array_default(), -1) + + +if __name__ == "__main__": + unittest.main() diff --git a/f90wrap/directc_cgen/arguments_scalar.py b/f90wrap/directc_cgen/arguments_scalar.py index 31d3d062e..610608d7c 100644 --- a/f90wrap/directc_cgen/arguments_scalar.py +++ b/f90wrap/directc_cgen/arguments_scalar.py @@ -103,6 +103,9 @@ def prepare_scalar_argument(gen: 'DirectCGenerator', arg: ft.Argument, intent: s gen.write(f"if (py_{arg.name} == Py_None) {{") gen.indent() gen.write(f"{arg.name}_val = 0;") + # Pass a NULL pointer so the Fortran side sees present(arg) == .false. + # instead of an argument whose value happens to be zero. + gen.write(f"{arg.name} = NULL;") gen.dedent() gen.write("} else {") gen.indent() @@ -121,7 +124,10 @@ def _prepare_character_none_case( """Handle None value for character arguments.""" gen.write(f"if (py_{arg.name} == Py_None) {{") gen.indent() - if optional or intent != "in": + if optional and not is_output_argument(arg): + # Absent optional input: pass a NULL pointer so present(arg) == .false. + gen.write(f"{arg.name} = NULL;") + elif optional or intent != "in": gen.write(f"{arg.name}_len = {default_len};") gen.write(f"if ({arg.name}_len <= 0) {{") gen.indent() From 1d7ccb1071931552c69c180b3e627bdc651449b5 Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Fri, 12 Jun 2026 16:09:22 +0200 Subject: [PATCH 2/3] fix: pass NULL for absent optional character outputs in --direct-c Optional intent(out)/inout character arguments still allocated a blank buffer when None was passed, so present(arg) stayed .true. Pass NULL for every absent optional character, and return None for a NULL buffer in the output path. The f2py and direct-C backends use different calling conventions for optional character outputs, so this case cannot share the example harness. Cover it with direct-C generation tests in test/test_directc.py, alongside checks for the optional scalar and optional character input. --- f90wrap/directc_cgen/arguments_scalar.py | 7 ++--- f90wrap/directc_cgen/procedures_return.py | 8 +++++- test/test_directc.py | 33 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/f90wrap/directc_cgen/arguments_scalar.py b/f90wrap/directc_cgen/arguments_scalar.py index 610608d7c..f88e71412 100644 --- a/f90wrap/directc_cgen/arguments_scalar.py +++ b/f90wrap/directc_cgen/arguments_scalar.py @@ -124,10 +124,11 @@ def _prepare_character_none_case( """Handle None value for character arguments.""" gen.write(f"if (py_{arg.name} == Py_None) {{") gen.indent() - if optional and not is_output_argument(arg): - # Absent optional input: pass a NULL pointer so present(arg) == .false. + if optional: + # Absent optional argument: pass a NULL pointer so present(arg) == .false. + # The output path returns None for a NULL character buffer. gen.write(f"{arg.name} = NULL;") - elif optional or intent != "in": + elif intent != "in": gen.write(f"{arg.name}_len = {default_len};") gen.write(f"if ({arg.name}_len <= 0) {{") gen.indent() diff --git a/f90wrap/directc_cgen/procedures_return.py b/f90wrap/directc_cgen/procedures_return.py index b7feb475a..6f9180f05 100644 --- a/f90wrap/directc_cgen/procedures_return.py +++ b/f90wrap/directc_cgen/procedures_return.py @@ -152,7 +152,13 @@ def _prepare_character_output(gen: DirectCGenerator, arg: ft.Argument) -> None: if parsed: # Check if buffer is from numpy array gen.write(f"PyObject* py_{arg.name}_obj = NULL;") - gen.write(f"if ({arg.name}_is_array) {{") + # An absent optional argument has a NULL buffer: return None for it. + gen.write(f"if ({arg.name} == NULL) {{") + gen.indent() + gen.write("Py_INCREF(Py_None);") + gen.write(f"py_{arg.name}_obj = Py_None;") + gen.dedent() + gen.write(f"}} else if ({arg.name}_is_array) {{") gen.indent() gen.write("/* Numpy array was modified in place, no return object or free needed */") gen.dedent() diff --git a/test/test_directc.py b/test/test_directc.py index 508b13138..63d5b5c0c 100644 --- a/test/test_directc.py +++ b/test/test_directc.py @@ -324,6 +324,39 @@ def test_c_code_syntax_basics(self): self.assertNotIn('def ', c_code) self.assertNotIn('import ', c_code) + def _generate_with_argument(self, name, type_spec, attributes): + arg = Mock(spec=ft.Argument) + arg.name = name + arg.type = type_spec + arg.attributes = attributes + self.generator.root.modules[0].procedures[0].arguments = [arg] + return self.generator.generate_module('testmod') + + def test_optional_scalar_passes_null(self): + """Issue #369: an absent optional scalar passes NULL, not a zeroed value.""" + c_code = self._generate_with_argument('n', 'integer', ['intent(in)', 'optional']) + + self.assertIn('if (py_n == Py_None)', c_code) + self.assertIn('n = NULL;', c_code) + + def test_optional_character_input_passes_null(self): + """An absent optional character input passes NULL so present() is .false.""" + c_code = self._generate_with_argument( + 'name', 'character(len=8)', ['intent(in)', 'optional'] + ) + + self.assertIn('name = NULL;', c_code) + + def test_optional_character_output_passes_null_and_returns_none(self): + """An absent optional character output passes NULL and is returned as None.""" + c_code = self._generate_with_argument( + 'label', 'character(len=8)', ['intent(out)', 'optional'] + ) + + self.assertIn('label = NULL;', c_code) + self.assertIn('if (label == NULL)', c_code) + self.assertIn('py_label_obj = Py_None;', c_code) + def test_c_code_has_character_setter(self): """Character module variables should generate setter wrappers.""" element = Mock(spec=ft.Element) From c4b40af6025d85a0f557469622b1acedef14a2ce Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Fri, 12 Jun 2026 20:36:06 +0200 Subject: [PATCH 3/3] Remove redundant zeroing of absent optional scalar in --direct-c The None branch overwrites the argument pointer with NULL before the Fortran call, so the preceding _val = 0 is never read. Drop it. --- f90wrap/directc_cgen/arguments_scalar.py | 1 - 1 file changed, 1 deletion(-) diff --git a/f90wrap/directc_cgen/arguments_scalar.py b/f90wrap/directc_cgen/arguments_scalar.py index f88e71412..0e8341dfc 100644 --- a/f90wrap/directc_cgen/arguments_scalar.py +++ b/f90wrap/directc_cgen/arguments_scalar.py @@ -102,7 +102,6 @@ def prepare_scalar_argument(gen: 'DirectCGenerator', arg: ft.Argument, intent: s if optional: gen.write(f"if (py_{arg.name} == Py_None) {{") gen.indent() - gen.write(f"{arg.name}_val = 0;") # Pass a NULL pointer so the Fortran side sees present(arg) == .false. # instead of an argument whose value happens to be zero. gen.write(f"{arg.name} = NULL;")