Skip to content

Conversation

@rwgk
Copy link
Contributor

@rwgk rwgk commented Mar 24, 2023

Description

1bcd8e8 is to adjust to smart_holder PR pybind/pybind11#4586

Note that the # SURPRISE! (see test_class_sh_property_stl.py) found while working on that commit exists already on master: pybind/pybind11#4591

Suggested changelog entry:

dependabot bot and others added 7 commits March 23, 2023 10:23
…ogle#4584)

Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.1 to 1.8.3.
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.8.1...v1.8.3)

---
updated-dependencies:
- dependency-name: pypa/gh-action-pypi-publish
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add test_class_sh_property_non_owning.cpp,py

Failing:

```
__________________________________________________________ test_persistent_holder __________________________________________________________

    def test_persistent_holder():
        h = m.DataFieldsHolder(2)
>       c = h.vec_at(0).core_fld
E       RuntimeError: Non-owning holder (loaded_as_shared_ptr).

h          = <pybind11_tests.class_sh_property_non_owning.DataFieldsHolder object at 0x7fabab516470>

test_class_sh_property_non_owning.py:6: RuntimeError
__________________________________________________________ test_temporary_holder ___________________________________________________________

    def test_temporary_holder():
        d = m.DataFieldsHolder(2).vec_at(1)
>       c = d.core_fld
E       RuntimeError: Non-owning holder (loaded_as_shared_ptr).

d          = <pybind11_tests.class_sh_property_non_owning.DataField object at 0x7fabab548770>

test_class_sh_property_non_owning.py:13: RuntimeError
```

* Introduce `shared_ptr_from_python(responsible_parent)` and use in all `property_cpp_function`s with `const shared_ptr<T> &` arguments.

Tests are incomplete.

* Complete tests.

* Add comment for `smart_holder_type_caster_load<T>::shared_ptr_from_python`
* use C++17 syntax to get rid of recursive template instantiations for concatenating type signatures (google#4587)

* Apply descr.h `src_loc` change (smart_holder PR google#4022) to code added with master PR google#4587

* Add test_class_sh_property_non_owning to CMakeLists.txt (fixes oversight in PR google#4586)

* Resolve clang-tidy errors.

* clang-tidy auto fix

---------

Co-authored-by: Konstantin Bespalov <kos5tya@yandex.ru>
@google-cla
Copy link

google-cla bot commented Mar 24, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

rwgk added 2 commits March 24, 2023 10:46
…ranch).

The behavior change made it possible to exercise the `reference_internal` vs `copy` behavior more thoroughly, which then led to a surprise discovery. See SURPRISE comment in tests/test_class_sh_property_stl.py
@rwgk rwgk force-pushed the pywrapcc_merge_sh branch from 930b971 to 1bcd8e8 Compare March 24, 2023 17:52
@rwgk rwgk marked this pull request as ready for review March 24, 2023 17:53
@rwgk rwgk requested a review from wangxf123456 March 24, 2023 17:55
@rwgk rwgk merged commit 53f48d3 into google:main Mar 24, 2023
@rwgk rwgk deleted the pywrapcc_merge_sh branch March 24, 2023 18:51
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.

3 participants