Skip to content

Fix constructor binding when function has optional arguments - #375

Open
krystophny wants to merge 3 commits into
jameskermode:masterfrom
krystophny:fix-issue-373-optional-constructor
Open

Fix constructor binding when function has optional arguments#375
krystophny wants to merge 3 commits into
jameskermode:masterfrom
krystophny:fix-issue-373-optional-constructor

Conversation

@krystophny

Copy link
Copy Markdown
Contributor

Fixes #373.

convert_derived_type_arguments locates the auto-generated ret_ return argument of a fortranconstructor by taking the first argument with the optional attribute. FunctionToSubroutineConverter inserts ret_ before the first optional argument, so when the constructor function has user-defined optional arguments the first optional is a user argument and the assertion sub.arguments[j].name[:4]=='ret_' fires. Find the ret_ argument by name prefix instead.

Adds examples/issue373_optional_constructor covering the interface widget_t constructor idiom with an optional argument, both present and absent.

Verification

Test fails on main

$ cd examples/issue373_optional_constructor && make test
  File ".../f90wrap/transform.py", line 562, in convert_derived_type_arguments
    assert(sub.arguments[j].name[:4]=='ret_')
AssertionError
make: *** [Makefile:24: f90wrap_main.f90] Error 1

Test passes after fix

$ cd examples/issue373_optional_constructor && make test
python tests.py
..
----------------------------------------------------------------------
Ran 2 tests in 0.000s

OK

Unit tests (pytest test/): 38 passed; the 2 pre-existing test_parse_dnad failures (missing test/samples/DNAD.fpp) are addressed separately. Related examples (keep_single_interface, interface, derivedtypes, fortran_oo, extends) all pass.

@jameskermode ready for review.

BindConstructorInterfaces marks constructor functions, and
convert_derived_type_arguments then moves the ret_ return argument to
the front. The lookup assumed the first optional argument is ret_,
which fails with AssertionError when the constructor has user-defined
optional arguments, since FunctionToSubroutineConverter inserts ret_
before the first optional argument. Find the ret_ argument by name
instead.

Fixes jameskermode#373
numpy 2.5 ships only manylinux_2_28 wheels, so the cp312 and cp313
Linux wheel builds fall back to compiling numpy from source inside the
default manylinux2014 container, where gcc 10.2.1 fails numpy's
'requires GCC >= 10.3' check. Build in the manylinux_2_28 image, whose
toolchain is new enough and where numpy wheels install directly.
@krystophny

Copy link
Copy Markdown
Contributor Author

Wheel CI failed for cp312/cp313 on Linux: numpy 2.5 ships only manylinux_2_28 wheels, so the default manylinux2014 container built numpy from source and failed its gcc >= 10.3 check. Included a one-line workflow fix building Linux wheels in the manylinux_2_28 image.

The new example calls the constructor without its optional argument,
which exposes the direct-c backend passing 0 instead of an absent
pointer, so present() is true and the default branch never runs. Apply
the direct-c optional-argument fix from jameskermode#370 so this PR is green
independently; the changes are identical, so either merge order is
clean.
@krystophny

Copy link
Copy Markdown
Contributor Author

The build (3.x) jobs failed in the direct-c pass: the new example calls the constructor without its optional argument, and on master the direct-c backend passes 0 instead of an absent pointer (#369), so present(scale) is true and the default branch never runs. Included the identical direct-c fix from #370 so this PR is green independently — either merge order is clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: BindConstructorInterfaces silently skips constructor when function has optional arguments

1 participant