Skip to content

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

Closed
vstinner wants to merge 1 commit intopython:mainfrom
vstinner:frozendict_fromkeys
Closed

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

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Feb 18, 2026

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

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

@corona10 corona10 left a comment

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
Member Author

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

Example:

class SpecialDict(frozendict):
    def __new__(self):
        return frozendict(x=None)

print(SpecialDict.fromkeys("y"))

Current output:

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

That's bad: the immutable frozendict is mutated!

@corona10
Copy link
Member

corona10 commented Feb 18, 2026

How about copy whole object at

else if (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
Member Author

How about copy whole object

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

@vstinner
Copy link
Member Author

I close this PR in favor of #144952 fix.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments