gh-145119: Allow frozendict to be assigned to instance __dict__ - #145123
gh-145119: Allow frozendict to be assigned to instance __dict__#145123mohsinm-dev wants to merge 2 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b702cf72b
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
| _PyObject_SetDict(PyObject *obj, PyObject *value) | ||
| { | ||
| if (value != NULL && !PyDict_Check(value)) { | ||
| if (value != NULL && !PyAnyDict_Check(value)) { |
There was a problem hiding this comment.
Avoid frozendict dict on objects mutating their own dict
Allowing _PyObject_SetDict() to accept PyAnyDict_Check here makes frozendict assignable to every PyObject_GenericSetDict user, but some of those types still perform internal writes with PyDict_SetItem/PyDict_DelItem (for example descriptor_get_wrapped_attribute and descriptor_set_wrapped_attribute in Objects/funcobject.c for classmethod/staticmethod annotation caching). PyDict_SetItem still rejects frozendict with PyErr_BadInternalCall, so after obj.__dict__ = frozendict(...), operations like reading or updating __annotations__ can now raise SystemError instead of behaving normally or reporting an intentional immutability error.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
|
In order to keep the commit history intact, please avoid squashing or amending history and then force-pushing to the PR. Reviewers often want to look at individual commits. When the PR is merged, everything will be squashed into a single commit. |
Sorry, something went wrong.
| Py_SETREF(dict, temp); | ||
| } | ||
| else if (PyFrozenDict_Check(dict)) { | ||
| /* Convert frozendict to a mutable dict for merging. */ |
There was a problem hiding this comment.
Please wait for #145517. If PyDict_Copy() is modified to always return a dict, you can reuse the PyDict_Check() branch for frozendict.
Sorry, something went wrong.
There was a problem hiding this comment.
For now, you can use _PyDict_CopyAsDict() from Include/internal/pycore_dict.h.
Sorry, something went wrong.
| "__dict__ must be set to a dictionary, " | ||
| "not a '%.200s'", Py_TYPE(value)->tp_name); |
There was a problem hiding this comment.
| "__dict__ must be set to a dictionary, " | |
| "not a '%.200s'", Py_TYPE(value)->tp_name); | |
| "__dict__ must be set to a dict or frozendict, " | |
| "not a %T", value); |
Sorry, something went wrong.
I extracted these changes from your PR and created a new PR based on it: PR gh-145564. |
Sorry, something went wrong.
|
The attached issue was closed. |
Sorry, something went wrong.
Allow frozendict (and its subclasses) to be assigned to an instance's __dict__, enabling immutable instances.
Changes: