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

Sync Py_TPFLAGS_MANAGED_DICT for PyPy3.11 across the codebase #5537

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Feb 20, 2025

Description

Adjust the Py_TPFLAGS_MANAGED_DICT logic in include/pybind11/attr.h to match the one used in include/pybind11/detail/class.h.

This is a followup to #5508.

CC @mattip

Adjust the `Py_TPFLAGS_MANAGED_DICT` logic in `include/pybind11/attr.h`
to match the one used in `include/pybind11/detail/class.h`.

This is a followup to pybind#5508.
@mattip
Copy link
Contributor

mattip commented Feb 20, 2025

Thanks, sorry I missed that in the previous PR.

@rwgk
Copy link
Collaborator

rwgk commented Feb 20, 2025

What do you think about a very slight more ambitious solution?

Untested:

In pybind11/detail/common.h:

#if PY_VERSION_HEX < 0x030B0000 || defined(PYPY_VERSION)
#define PYBIND11_BACKWARD_COMPATIBILITY_TP_DICTOFFSET
#endif

Then use that in the two places that need to stay in sync.

I'm not sure it's the best name for the define, please use what makes the most sense to you.

@mgorny
Copy link
Contributor Author

mgorny commented Feb 20, 2025

I'll try and push it, if it works.

@henryiii henryiii merged commit d8565ac into pybind:master Feb 20, 2025
79 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 20, 2025
@mgorny
Copy link
Contributor Author

mgorny commented Feb 21, 2025

Thanks!

@mgorny mgorny deleted the pypy311-dict-sync branch February 21, 2025 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Possibly needs a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants