gh-141510: Add frozendict_check_mutable()#144944
gh-141510: Add frozendict_check_mutable()#144944vstinner wants to merge 2 commits intopython:mainfrom
Conversation
frozendict.fromkeys() now checks if it can mutate the newly created frozendict.
|
@methane @corona10: I wrote this change to raise an exception if a frozendict subclass constructor keeps a reference to the newly created dictionary. I don't know which error message should be used, so I wrote: RuntimeError("cannot mutate frozendict already exposed in Python"). Do you have a better error message to propose? |
|
Frozen dicts are immutable, that's the whole point of them, so why are we allowing them to be mutated? Mutation of tuples has been an endless source of bugs, please don't do the same with frozen dicts. |
This should be a |
I concur with Mark's opinion here. |
| static int | ||
| frozendict_check_mutable(PyObject *self) | ||
| { | ||
| if (Py_REFCNT(self) > 1) { |
There was a problem hiding this comment.
IIRC, Checking Py_REFCNT at runtime is not thread safe at free-threaded build.
|
This change fix the following example: fd = None
class SpecialDict(frozendict):
def __new__(self):
global fd
fd = frozendict(x=1)
return fd
print(SpecialDict.fromkeys("y"))Currently, This change detects the special constructor and raises an error.
An alternative is to modify A more radical fix would be to disallow subclassing |
|
frozendict_check_mutable() was a bad idea, I close the PR. I wrote a 3rd PR to fix fromkeys(): #144952 copies the frozendict. |
frozendict.fromkeys() now checks if it can mutate the newly created frozendict.