fix: pass NULL for absent optional arguments in --direct-c#370
fix: pass NULL for absent optional arguments in --direct-c#370krystophny wants to merge 3 commits into
Conversation
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 jameskermode#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.
|
@jameskermode could you review this? @GeorgGrassler this fixes the optional-argument |
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.
|
Thanks for the report and fix. Looks fine from a quick read, |
| @@ -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;") | |||
There was a problem hiding this comment.
@krystophny @jameskermode yes it works now thanks. Curious about the still present {arg.name}_val = 0. Is this not now redundant?
There was a problem hiding this comment.
Good catch, you're right. The pointer is overwritten with NULL before the Fortran call, so _val = 0 was never read. Removed in c4b40af. The optional character-input branch was already clean (sets the pointer to NULL with no zeroing).
The None branch overwrites the argument pointer with NULL before the Fortran call, so the preceding _val = 0 is never read. Drop it.
Summary
Closes #369.
With
--direct-c, passingNonefor anOPTIONALargument left a non-NULL pointer reaching Fortran, sopresent(arg)returned.true.and any default-value branch was skipped. Three argument forms were affected:&<arg>_valafter the value was zeroed (the reported case)out/inout): a blank buffer was allocated, so an omitted optional was seen as presentAll now pass
NULL, matching the f2py path (#var#_capi == Py_None ? NULL : &#var#). For optional character outputs, the return path yieldsNonewhen the buffer isNULL.Audit of the remaining optional kinds in
directc_cgen: optional arrays already default the pointer toNULLand guard preparation with!= Py_None, and optional derived types initialise the handle pointer toNULLand fill it only when notNone. Both were already correct; the new tests lock that in.Verification
examples/issue369_optional_presentcovers optional scalar, character input, and array arguments through both build paths (f2py and--direct-c). Optional character outputs use different calling conventions on the two backends, so they are covered by direct-C generation tests intest/test_directc.pyinstead.Direct-C example, before the fix:
After the fix:
examples/issue369_optional_present: 6 tests OK on both the f2py and--direct-cpathstest/test_directc.py: 32 tests OK, including the optional scalar / character input / character output NULL checksAll tests passed(55 examples)