Skip to content

Add a SYMBOL_FUNCBASE_TYPES constant to avoid multiple-inheritance bug - #6481

Merged
msullivan merged 2 commits into
masterfrom
fix-mypyc-good
Feb 28, 2019
Merged

msullivan merged 2 commits into
masterfrom
fix-mypyc-good

Conversation

@msullivan

Copy link
Copy Markdown
Collaborator

A mypy bug (#3603) causes mypy to think that SymbolNodes cannot be
FuncBases, which causes mypy-mypyc to crash in code that tests
that.
Currently we work around this in a couple gross ways (like turning the
SymbolNode into Any). Work around it instead by testing directly
against a tuple of the subtypes instead of FuncBase itself.

This is the better fix promised in #6480.

…sues

A mypy bug (#3603) causes mypy to think that SymbolNodes cannot be
FuncBases, which causes mypy-mypyc to crash in code that tests
that.
Currently we work around this in a couple gross ways (like turning the
SymbolNode into Any). Work around it instead by testing directly
against a tuple of the subtypes instead of FuncBase itself.

This is the better fix promised in #6480.

@gvanrossum gvanrossum 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.

LG, though I'm lukewarm on how much clarity this adds...

Comment thread mypy/checkmember.py
super_info = node_node.info
if isinstance(node.node, Decorator):
super_info = node.node.var.info # type: Optional[TypeInfo]
elif isinstance(node.node, (Var, SYMBOL_FUNCBASE_TYPES)):

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.

So the hack now is that this is a variable and the binder doesn't know whether it's true or false?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There's no hack, anymore, really. We're just explicitly enumerating the overlap between SymbolNode and FuncBase instead of using isinstance on FuncBase

Comment thread mypy/nodes.py
cannot be a FuncBase.

Instead, test against SYMBOL_FUNCBASE_TYPES, which enumerates
SymbolNode subclasses that are also FuncBase subclasses.

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.

Sounds tedious. :-(

Comment thread mypy/nodes.py

# All types that are both SymbolNodes and FuncBases. See the FuncBase
# docstring for the rationale.
SYMBOL_FUNCBASE_TYPES = (OverloadedFuncDef, FuncDef)

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.

How do we ensure this stays up to date? If a new class were to be added, would tests for that new class fail clearly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

They would fail, I think, though I'm not sure how cleanly.

Comment thread mypy/nodes.py Outdated
@msullivan
msullivan merged commit 2482d0b into master Feb 28, 2019
@msullivan
msullivan deleted the fix-mypyc-good branch February 28, 2019 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants