Skip to content

gh-141510: Fix frozendict.fromkeys() for subclasses - #144947

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:frozendict_fromkeys
Closed

gh-141510: Fix frozendict.fromkeys() for subclasses#144947
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:frozendict_fromkeys

Conversation

@vstinner

@vstinnervstinner commented Feb 18, 2026

Copy link
Copy Markdown
Member

Don't call the constructor of the subclass, but frozendict constructor instead.

Don't call the constructor of the subclass, but frozendict
constructor instead.

@corona10corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should discuss this before taking any action here.
Do we really need to handle cases where users shoot themselves in the foot? It doesn’t cause a crash at the moment, right?

@vstinner

Copy link
Copy Markdown
MemberAuthor

Currently, frozendict.fromkeys() allows modifying an immutable frozendict using a special subclass.

Example:

classSpecialDict(frozendict):
def__new__(self):
returnfrozendict(x=None)
print(SpecialDict.fromkeys("y"))

Current output:

frozendict({'x': None, 'y': None})

That's bad: the immutable frozendict is mutated!

@corona10

corona10 commented Feb 18, 2026

Copy link
Copy Markdown
Member

How about copy whole object at

elseif (PyFrozenDict_CheckExact(d)) {
not to mutate original frozendict simply?

It follows the original intention of immutable data structures: copy instead of mutating.

@vstinner
vstinner marked this pull request as draft February 18, 2026 13:03
@vstinner

Copy link
Copy Markdown
MemberAuthor

How about copy whole object

Ok, good idea: I wrote #144952 to copy the frozendict.

@vstinner

Copy link
Copy Markdown
MemberAuthor

I close this PR in favor of #144952 fix.

@vstinner
vstinner deleted the frozendict_fromkeys branch February 18, 2026 15:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@corona10