Skip to content

Extend return_value_policy_pack support to py::cast() and trampolines #30015

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

Merged
merged 17 commits into from
Mar 20, 2023

Conversation

wangxf123456
Copy link
Contributor

@wangxf123456 wangxf123456 commented Mar 17, 2023

Description

Follow-on to #30011, to make the return_value_policy_pack support more complete:

  • py::cast()
  • macros for trampolines: PYBIND11_OVERRIDE_NAME_RVPP, PYBIND11_OVERRIDE_PURE_NAME_RVPP

Suggested changelog entry:

wangxf123456 and others added 15 commits March 17, 2023 13:43
…constexpr` for `return_value_policy` shortcuts.
Resolves clang-tidy errors: `'virtual' is redundant since the function is already declared 'override'`
… rely on __VA_OPT__

__VA_OPT__ is a C++20 feature that is supported only by some pre-C++20 compilers but not all.

The dangling commas (/* no arguments */) in the PYBIND11_OVERRIDE_.*NAME_RVPP invocations need to be removed.
@rwgk rwgk added the gha_wf_python312dev_enable Enable python312dev.yml label Mar 19, 2023
rwgk added 2 commits March 19, 2023 09:41
…ERRIDE_PURE_NAME, PYBIND11_OVERRIDE_PURE_NAME_RVPP
@rwgk rwgk changed the title Follow up of return_value_policy_pack Extend return_value_policy_pack support to py::cast and trampolines Mar 19, 2023
@rwgk rwgk changed the title Extend return_value_policy_pack support to py::cast and trampolines Extend return_value_policy_pack support to py::cast() and trampolines Mar 19, 2023
@rwgk rwgk marked this pull request as ready for review March 19, 2023 20:22
@wangxf123456
Copy link
Contributor Author

The changes look good to me.

@rwgk rwgk merged commit 43df8a4 into google:main Mar 20, 2023
lanctot pushed a commit to google-deepmind/open_spiel that referenced this pull request Mar 27, 2023
`PYBIND11_OVERRIDE_IMPL` is an implementation detail and not meant to be used outside the pybind11 source tree.

Notes:

* For easy reference: `PYBIND11_OVERRIDE_IMPL` was added to python_games.cc in cl/379240506.

* Discovered in connection with google/pybind11clif#30015, which changes `PYBIND11_OVERRIDE_IMPL`. (The pywrapcc fork of pybind11 is not currently used in production, but we're using it regularly to run TAP Global Presubmits, to test go/pyclif_pybind11_fusion developments.)

* FYI: Test coverage seems to be incomplete: control flow passes through the changed code for many tests, but replacing `"mean_field_population"` with `"XXXmean_field_population"` does not break any tests (see isolated TGP results under http://tap/OCL:517999370:BASE:517998831:1679332509158:a5f7412a). I.e. a test with a Python override is missing.

* FYI: The `PYBIND11_OVERLOAD_*` macros used in other parts of python_games.cc are deprecated since Sep 2020 (pybind/pybind11#2325). It is recommended to replace them with the equivalent `PYBIND11_OVERRIDE_*` macros.

PiperOrigin-RevId: 518266312
Change-Id: Ica90764a71b8ed7795b2f26ab02a904bf2ad901e
@wangxf123456 wangxf123456 deleted the return_value_policy_pack_followup branch May 18, 2023 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gha_wf_python312dev_enable Enable python312dev.yml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants