Add support for PyFrozenDict by MatthieuDartiailh · Pull Request #6174 · PyO3/pyo3

MatthieuDartiailh · 2026-06-30T21:19:28Z

No description provided.

MatthieuDartiailh · 2026-07-02T21:29:06Z

I am a bit at a loss when it comes to the ffi checks. Pointers welcome

alex · 2026-07-02T21:31:37Z

AFAIK the header that contains this in CPython is Include/internal/pycore_dict.h, so you'd need to include that path.

(Not opining on any part of this patch.)

MatthieuDartiailh · 2026-07-02T21:55:00Z

Thanks I will try that.

MatthieuDartiailh · 2026-07-03T06:47:09Z

So the ffi-check situtation is a bit more complex since accessing the core headers requires Py_BUILD_CORE define. Is this something pyo3 maintainers would be willing to accept ?

Icxolu

I'm also not an expert on the FFI layer, but here are some comment from my side.

(I haven't looked at the wrapping code, as we first need to get the FFI bindings correct.)

davidhewitt · 2026-07-04T09:31:15Z

So the ffi-check situtation is a bit more complex since accessing the core headers requires Py_BUILD_CORE define. Is this something pyo3 maintainers would be willing to accept ?

We should not be using any symbols from Include/internal.

MatthieuDartiailh · 2026-07-06T22:04:13Z

~~I think this should now look better. I squashed the previous change to reduce the noise.~~ so running just the lib test was too light. I will continue tomorrow.

Icxolu

Thank you, I think we are getting closer. I went through the safe wrapper as well and left some comments below.

Icxolu · 2026-07-07T18:55:03Z

+#[inline]
+#[cfg(Py_3_15)]
+pub unsafe fn PyAnyDict_CheckExact(op: *mut PyObject) -> c_int {
+    (Py_TYPE(op) == &raw mut PyDict_Type || Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int


Suggested change

(Py_TYPE(op) == &raw mut PyDict_Type || Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int

(PyDict_CheckExact(op) != 0 || PyFrozenDict_CheckExact(op) != 0) as c_int

To be consistent I think we should so this instead.

MatthieuDartiailh · 2026-07-08T05:02:03Z

Thanks for the review. I have been fighthing a bit cfg when it comes to the limited API. I will have to look into your suggestions.

MatthieuDartiailh · 2026-07-08T21:05:07Z

I did a first pass in addressing the review and got clean clippy and test execution locally. The wrapper is now available without the limited API and the cfg are a bit more tiddy. I will check the CI run tomorrow and see if I missed anything else.

codspeed-hq · 2026-07-08T21:19:23Z

Merging this PR will not alter performance

✅ 140 untouched benchmarks

_{Comparing MatthieuDartiailh:frozendict (85acaf7) with main (8fcf8fc)}

Icxolu

Thank you, some smaller suggestions and some more cfg massaging is needed, but not too much more until we can land this I would say.

…ro exclusion list

MatthieuDartiailh · 2026-07-11T08:36:29Z

Thanks for the early reviews @Icxolu

I think I now have everything in order (including the cfgs).

Icxolu

Thank you, looking good! Just one small comment regarding the constructor after which I think this is good to go.

Icxolu · 2026-07-11T13:35:16Z

    pub fn new<'py, T>(py: Python<'py>, iterable: T) -> PyResult<Bound<'py, PyFrozenDict>>
    where
        T: IntoPyObject<'py>,
        err::PyErr: core::convert::From<<T as conversion::IntoPyObject<'py>>::Error>,


Typically the new constructor for similar types takes a Rust iterator. See for example PyFrozenSet or PyTuple. To be consistent I think we should follow that here as well.

I can see that it could be useful to build a PyFrozenDict directly from a Python. The equivalent on dict is called from_sequence, we can use the same here. Similarly we should not make that generic to make users aware that they need to check what they are passing in.

I also saw that for PyFrozenSet we have a PyFrozenSetBuilder. This could be useful for PyFrozenDict as well, out of scope for this PR, but could be a possible followup.

Icxolu · 2026-07-11T13:38:13Z

+    }
+}
+
+#[cfg(all(Py_3_15, test))]


Suggested change

#[cfg(all(Py_3_15, test))]

#[cfg(test)]

Icxolu · 2026-07-11T13:39:38Z

#[cfg(Py_3_15)] on all entries should be enough here.

bschoenmaeckers · 2026-07-15T08:19:52Z

As a possible followup we can add support for a PyAnyDict, where both PyDict & PyFrozenDict have a Deref impl to PyAnyDict. This will remove some duplications from both types.

bschoenmaeckers · 2026-07-15T08:25:21Z

+#[inline]
+#[cfg(Py_3_15)]
+pub unsafe fn PyFrozenDict_CheckExact(op: *mut PyObject) -> c_int {
+    (Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int


Suggested change

(Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int

Py_IS_TYPE(op, &raw mut PyFrozenDict_Type)

bschoenmaeckers · 2026-07-15T08:29:09Z

+    (Py_TYPE(op) == &raw mut PyFrozenDict_Type
+        || PyType_IsSubtype(Py_TYPE(op), &raw mut PyFrozenDict_Type) != 0) as c_int


Suggested change

(Py_TYPE(op) == &raw mut PyFrozenDict_Type

|| PyType_IsSubtype(Py_TYPE(op), &raw mut PyFrozenDict_Type) != 0) as c_int

PyObject_TypeCheck(op, &raw mut PyFrozenDict_Type)

MatthieuDartiailh force-pushed the frozendict branch 2 times, most recently from ee55a45 to fd12ac9 Compare July 2, 2026 20:05

Icxolu reviewed Jul 3, 2026

View reviewed changes

Comment thread pyo3-ffi/src/cpython/dictobject.rs Outdated

Comment thread pyo3-ffi/src/dictobject.rs Outdated

MatthieuDartiailh mentioned this pull request Jul 6, 2026

Provide Rust wrappers around PyFrozenDict #6158

Open

Icxolu reviewed Jul 6, 2026

View reviewed changes

Comment thread pyo3-ffi-check/macro/src/lib.rs Outdated

add support for PyFrozenDict

b3a52c3

MatthieuDartiailh force-pushed the frozendict branch from 27e8799 to b3a52c3 Compare July 6, 2026 22:02

ffi: fix cfg gate on import

2618d5a

Icxolu requested changes Jul 7, 2026

View reviewed changes

address review comments

9ac0edd

Icxolu reviewed Jul 8, 2026

View reviewed changes

Comment thread src/types/frozendict.rs Outdated

Comment thread src/types/frozendict.rs Outdated

Comment thread src/types/frozendict.rs Outdated

Comment thread src/types/frozendict.rs Outdated

MatthieuDartiailh added 4 commits July 9, 2026 17:19

address review comments

97afbe1

add PyAnyDict_Check and PyFrozenDict_Check family of macro to the mac…

468c7ad

…ro exclusion list

fix clippy

e84426c

more clippy fixes

0373aee

MatthieuDartiailh force-pushed the frozendict branch from 4efcac1 to 0373aee Compare July 9, 2026 20:53

MatthieuDartiailh added 5 commits July 10, 2026 19:15

add missing conditional import

ac163c5

missing trait

78ffcbd

bad cfg fix

f3ba28a

fix new frozendict creation

604a996

remove useless import

9aa40cb

trim useless docstring

85acaf7

MatthieuDartiailh marked this pull request as ready for review July 10, 2026 22:34

Icxolu reviewed Jul 11, 2026

View reviewed changes

bschoenmaeckers requested changes Jul 15, 2026

View reviewed changes

	(Py_TYPE(op) == &raw mut PyDict_Type \|\| Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int
	(PyDict_CheckExact(op) != 0 \|\| PyFrozenDict_CheckExact(op) != 0) as c_int

	(Py_TYPE(op) == &raw mut PyFrozenDict_Type) as c_int
	Py_IS_TYPE(op, &raw mut PyFrozenDict_Type)

		(Py_TYPE(op) == &raw mut PyFrozenDict_Type
		\|\| PyType_IsSubtype(Py_TYPE(op), &raw mut PyFrozenDict_Type) != 0) as c_int

	(Py_TYPE(op) == &raw mut PyFrozenDict_Type
	\|\| PyType_IsSubtype(Py_TYPE(op), &raw mut PyFrozenDict_Type) != 0) as c_int
	PyObject_TypeCheck(op, &raw mut PyFrozenDict_Type)

Uh oh!

Conversation

MatthieuDartiailh commented Jun 30, 2026

Uh oh!

MatthieuDartiailh commented Jul 2, 2026

Uh oh!

alex commented Jul 2, 2026

Uh oh!

MatthieuDartiailh commented Jul 2, 2026

Uh oh!

MatthieuDartiailh commented Jul 3, 2026

Uh oh!

Icxolu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davidhewitt commented Jul 4, 2026

Uh oh!

Uh oh!

MatthieuDartiailh commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Icxolu left a comment

Choose a reason for hiding this comment

Uh oh!

Icxolu Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatthieuDartiailh commented Jul 8, 2026

Uh oh!

MatthieuDartiailh commented Jul 8, 2026

Uh oh!

codspeed-hq Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

Icxolu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatthieuDartiailh commented Jul 11, 2026

Uh oh!

Icxolu left a comment

Choose a reason for hiding this comment

Uh oh!

Icxolu Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

Icxolu Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

Icxolu Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

bschoenmaeckers commented Jul 15, 2026

Uh oh!

bschoenmaeckers Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

bschoenmaeckers Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

MatthieuDartiailh commented Jul 6, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 8, 2026 •

edited

Loading