Skip to content

gh-143831: Compare cells by identity in forward references #143848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
johnslavik wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −2
Draft

gh-143831: Compare cells by identity in forward references #143848

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Lib/annotationlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ def __eq__(self, other):
# because dictionaries are not hashable.
and self.__globals__ is other.__globals__
and self.__forward_is_class__ == other.__forward_is_class__
and self.__cell__ == other.__cell__
# Two separate cells are always considered unequal in forward refs.
and self.__cell__ is other.__cell__
and self.__owner__ == other.__owner__
and (
(tuple(sorted(self.__extra_names__.items())) if self.__extra_names__ else None) ==
Expand All @@ -293,7 +294,10 @@ def __hash__(self):
self.__forward_module__,
id(self.__globals__), # dictionaries are not hashable, so hash by identity
self.__forward_is_class__,
tuple(sorted(self.__cell__.items())) if isinstance(self.__cell__, dict) else self.__cell__,
( # cells are mutable and not hashable as well
tuple(sorted([(name, id(cell)) for name, cell in self.__cell__.items()]))
Copy link

@abhinand-c abhinand-c Jan 15, 2026

Choose a reason for hiding this comment

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

@johnslavik Do we need the list comprehension here? We could pass a generator directly to sorted() for slightly better efficiency:

tuple(sorted((name, id(cell)) for name, cell in self.__cell__.items()))

Also, is there any way to avoid sorting altogether without compromising the stability or correctness of the hash?

Copy link
Member Author

@johnslavik johnslavik Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need the list comprehension here? We could pass a generator directly to sorted() for slightly better efficiency:

tuple(sorted((name, id(cell)) for name, cell in self.__cell__.items()))

This is not necessarily more efficient.

List comprehensions are inlined since PEP 709. I've used a list comprehension on purpose here because cell dicts would never be big -- walking a new transient generator on small data is more expensive than computing an eager list. See also #143825 (comment).

Also, is there any way to avoid sorting altogether without compromising the stability or correctness of the hash?

How does sorting here compromise stability or correctness?

This code was meant to sort cell dicts. Although this sounds like an unlikely situation, I think it makes sense for hashes of two different cell dicts that happen to point to identical cells to be the same.

if isinstance(self.__cell__, dict) else id(self.__cell__),
),
self.__owner__,
tuple(sorted(self.__extra_names__.items())) if self.__extra_names__ else None,
))
Expand Down
Loading