-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
gh-129463, gh-128593: Simplify ForwardRef #129465
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.
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
Changes from 11 commits
060ca69
9c1c8c8
5d06ec5
f59d4fb
2b26af0
fbb281f
3bbf940
fe665f7
d273c05
34c8ad5
46593f8
e71eecf
0bb5bc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,6 @@ class Format(enum.IntEnum): | |
# preserved for compatibility with the old typing.ForwardRef class. The remaining | ||
# names are private. | ||
_SLOTS = ( | ||
"__forward_evaluated__", | ||
"__forward_value__", | ||
"__forward_is_argument__", | ||
"__forward_is_class__", | ||
"__forward_module__", | ||
|
@@ -76,8 +74,6 @@ def __init__( | |
raise TypeError(f"Forward reference must be a string -- got {arg!r}") | ||
|
||
self.__arg__ = arg | ||
self.__forward_evaluated__ = False | ||
self.__forward_value__ = None | ||
self.__forward_is_argument__ = is_argument | ||
self.__forward_is_class__ = is_class | ||
self.__forward_module__ = module | ||
|
@@ -95,17 +91,11 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None): | |
|
||
If the forward reference cannot be evaluated, raise an exception. | ||
""" | ||
if self.__forward_evaluated__: | ||
return self.__forward_value__ | ||
if self.__cell__ is not None: | ||
try: | ||
value = self.__cell__.cell_contents | ||
return self.__cell__.cell_contents | ||
except ValueError: | ||
pass | ||
else: | ||
self.__forward_evaluated__ = True | ||
self.__forward_value__ = value | ||
return value | ||
if owner is None: | ||
owner = self.__owner__ | ||
|
||
|
@@ -171,8 +161,6 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None): | |
else: | ||
code = self.__forward_code__ | ||
value = eval(code, globals=globals, locals=locals) | ||
self.__forward_evaluated__ = True | ||
self.__forward_value__ = value | ||
return value | ||
|
||
def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard): | ||
|
@@ -230,18 +218,30 @@ def __forward_code__(self): | |
def __eq__(self, other): | ||
if not isinstance(other, ForwardRef): | ||
return NotImplemented | ||
if self.__forward_evaluated__ and other.__forward_evaluated__: | ||
return ( | ||
self.__forward_arg__ == other.__forward_arg__ | ||
and self.__forward_value__ == other.__forward_value__ | ||
) | ||
return ( | ||
self.__forward_arg__ == other.__forward_arg__ | ||
and self.__forward_module__ == other.__forward_module__ | ||
and self.__forward_is_class__ == other.__forward_is_class__ | ||
and self.__code__ == other.__code__ | ||
and self.__ast_node__ == other.__ast_node__ | ||
# Use "is" here because we use id() for this in __hash__ | ||
# because dictionaries are not hashable. | ||
and self.__globals__ is other.__globals__ | ||
and self.__cell__ == other.__cell__ | ||
and self.__owner__ == other.__owner__ | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could possibly micro-optimise this slightly by moving some of the cheaper comparisons up (e.g. the It took me a while to wrap my head around the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reordered them a bit. I think the performance effects are quite hard to predict though because it matters not just how expensive the check is, but also how likely it is to short-circuit the comparison. For example, maybe since we already checked the module, the globals check actually never makes a difference, because if the module doesn't match the globals also won't match. If somebody wants to micro-optimize this I won't stop them but I'd rather just land this; I doubt it will make a huge difference either way. |
||
|
||
def __hash__(self): | ||
return hash((self.__forward_arg__, self.__forward_module__)) | ||
return hash(( | ||
self.__forward_arg__, | ||
self.__forward_module__, | ||
self.__forward_is_class__, | ||
self.__code__, | ||
self.__ast_node__, | ||
id(self.__globals__), # dictionaries are not hashable, so hash by identity | ||
self.__cell__, | ||
self.__owner__, | ||
)) | ||
|
||
def __or__(self, other): | ||
return types.UnionType[self, other] | ||
|
@@ -250,11 +250,14 @@ def __ror__(self, other): | |
return types.UnionType[other, self] | ||
|
||
def __repr__(self): | ||
if self.__forward_module__ is None: | ||
module_repr = "" | ||
else: | ||
module_repr = f", module={self.__forward_module__!r}" | ||
return f"ForwardRef({self.__forward_arg__!r}{module_repr})" | ||
extra = [] | ||
if self.__forward_module__ is not None: | ||
extra.append(f", module={self.__forward_module__!r}") | ||
if self.__forward_is_class__: | ||
extra.append(", is_class=True") | ||
if self.__owner__ is not None: | ||
extra.append(f", owner={self.__owner__!r}") | ||
return f"ForwardRef({self.__forward_arg__!r}{''.join(extra)})" | ||
|
||
|
||
class _Stringifier: | ||
|
@@ -276,8 +279,6 @@ def __init__( | |
# represent a single name). | ||
assert isinstance(node, (ast.AST, str)) | ||
self.__arg__ = None | ||
self.__forward_evaluated__ = False | ||
self.__forward_value__ = None | ||
self.__forward_is_argument__ = False | ||
self.__forward_is_class__ = is_class | ||
self.__forward_module__ = None | ||
|
Uh oh!
There was an error while loading. Please reload this page.