[mypyc] Don't load forward ref targets while setting up non-ext __annotations__ #14401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ichard26
force-pushed
the
fix-fwdref-in-class-based-namedtuple-annotations-no-future
branch
from
5282a98
to
a492135
Compare
Take this example:
from typing import NamedTuple
class VTableMethod(NamedTuple):
cls: "ClassIR"
class ClassIR: pass
In irbuild::classdef::add_non_ext_class_attr_ann(), mypyc tries to
assign the ClassIR type object to VTableMethod's __annotations__. This
causes a segfault as ClassIR won't be initialized and allocated until
*after* the NamedTuple is set up.
Fortunately, AssignmentStmt preserves the unanalyzed type (UnboundType).
If `stmt.unanalyzed_type.orginal_str_expr` is not None, then we know
we're dealing with a forward ref and should just load the string
instead.
Unfortunately, it seems difficult (or impossible?) to infer whether
an annotation is a forward reference when the annotations future is
enabled and the annotation isn't a string.
ichard26
force-pushed
the
fix-fwdref-in-class-based-namedtuple-annotations-no-future
branch
from
a492135
to
b867e24
Compare
JukkaL
approved these changes
Jan 9, 2023
| # type and load the string instead. | ||
| # | ||
| # TODO: is it possible to determine whether a non-string annotation is | ||
| # actually a forward reference due to the __annotations__ future? |
There was a problem hiding this comment.
If from __future__ import annotations is used, I think that we can always use string annotations, or whenever there is any doubt that type might contain forward references.
There was a problem hiding this comment.
Ah yeah you're right. I'll open an issue about hardening our __annotations__ handling overall when I have time.
|
@hauntsaninja you can try using class-based NamedTuples again now :) |
ichard26
deleted the
fix-fwdref-in-class-based-namedtuple-annotations-no-future
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Take this example:
In irbuild::classdef::add_non_ext_class_attr_ann(), mypyc tries to assign the ClassIR type object to VTableMethod's __annotations__. This causes a segfault as ClassIR won't be initialized and allocated until after the NamedTuple is set up.
Fortunately, AssignmentStmt preserves the unanalyzed type (UnboundType). If
stmt.unanalyzed_type.orginal_str_expris not None, then we know we're dealing with a forward ref and should just load the string instead.Unfortunately, it seems difficult (or impossible?) to infer whether an annotation is a forward reference when the annotations future is enabled and the annotation isn't a string.
I guess this fixes mypyc/mypyc#938?
Dataclasses are still broken as strings in __annotations__ cause dataclass's internals to lookup the module in sys.modules which fails (as the module hasn't been fully initialized and added to sys.modules yet!)