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 1 commit intopython:mainfrom
Conversation
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 GitHub
Your 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".
| _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.
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 👍 / 👎.
6b702cf to
8d6ec8d
Compare
|
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. |
| 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.
| "__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); |
Allow
frozendict(and its subclasses) to be assigned to an instance's__dict__, enabling immutable instances.Changes:
_PyObject_SetDict: acceptfrozendictviaPyAnyDict_Check_PyDict_SetItem_LockHeld: raiseTypeErroron set/delete instead ofSystemErrorobject___dir___impl: convertfrozendictto a mutable dict for merging