Skip to content
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

testing smart_holder as default #2872

Closed
wants to merge 207 commits into from
Closed

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Feb 24, 2021

Same as #2672 but with PYBIND11_USE_SMART_HOLDER_AS_DEFAULT defined.

Ralf W. Grosse-Kunstleve added 30 commits February 23, 2021 13:00
See also: pybind#2583

Does not build with upstream master or
pybind#2047, but builds with
https://github.com/RobotLocomotion/pybind11 and almost runs:

```
Running tests in directory "/usr/local/google/home/rwgk/forked/EricCousineau-TRI/pybind11/tests":
================================================================================= test session starts =================================================================================
platform linux -- Python 3.8.5, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /usr/local/google/home/rwgk/forked/EricCousineau-TRI/pybind11/tests, inifile: pytest.ini
collected 2 items

test_unique_ptr_member.py .F                                                                                                                                                    [100%]

====================================================================================== FAILURES =======================================================================================
_____________________________________________________________________________ test_pointee_and_ptr_owner ______________________________________________________________________________

    def test_pointee_and_ptr_owner():
        obj = m.pointee()
        assert obj.get_int() == 213
        m.ptr_owner(obj)
        with pytest.raises(ValueError) as exc_info:
>           obj.get_int()
E           Failed: DID NOT RAISE <class 'ValueError'>

test_unique_ptr_member.py:17: Failed
============================================================================= 1 failed, 1 passed in 0.06s =============================================================================
```
…pecialization to make the bindings involving unique_ptr passing compile, but load and cast implementations are missing
https://github.com/rwgk/pybind11/tree/xxx_value_ptr_xxx_holder

Systematically exercising returning and passing unique_ptr<T>, shared_ptr<T>
with unique_ptr, shared_ptr holder.

Observations:

test_holder_unique_ptr:
  make_unique_pointee  OK
  pass_unique_pointee  BUILD_FAIL (as documented)
  make_shared_pointee  Abort free(): double free detected
  pass_shared_pointee  RuntimeError: Unable to load a custom holder type from a default-holder instance

test_holder_shared_ptr:
  make_unique_pointee  Segmentation fault (pybind#1138)
  pass_unique_pointee  BUILD_FAIL (as documented)
  make_shared_pointee  OK
  pass_shared_pointee  OK
https://github.com/rwgk/pybind11/tree/xxx_value_ptr_xxx_holder

Systematically exercising casting between shared_ptr<base>, shared_ptr<derived>.
(This demo does NOT involve smart pointers at all, unlike the otherwise similar test_smart_ptr_private_first_base.)
…hat this can be wrapped with Boost.Python, using boost::noncopyable)
Ralf W. Grosse-Kunstleve added 22 commits February 23, 2021 13:00
…ags (only the throw are not actually exercised, investing time there has a high cost but very little benefit).
…everal holders.

Tested using github.com/rwgk/pybind11_scons and Google-internal build system.
Sorry, no cmake support at the moment.

First results: https://docs.google.com/spreadsheets/d/1InapCYws2Gt-stmFf_Bwl33eOMo3aLE_gc9adveY7RU/edit#gid=0
…_uses_smart_holder_type_caster for clarity.
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 24, 2021

PR #2672 was squash-merged into the new smart_holder branch.

@rwgk rwgk closed this Feb 24, 2021
@rwgk rwgk deleted the smart_holder_as_default branch February 24, 2021 05:52
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.

1 participant