Skip to content
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

Forward references to NamedTuples don't behave correctly in some instances #2762

Closed
axch opened this issue Jan 26, 2017 · 2 comments · Fixed by #3952
Closed

Forward references to NamedTuples don't behave correctly in some instances #2762

axch opened this issue Jan 26, 2017 · 2 comments · Fixed by #3952
Labels
bug mypy got something wrong topic-named-tuple

Comments

@axch
Copy link

axch commented Jan 26, 2017

Please forgive me if this is an ignorant question, but I've looked around a bit and can't seem to find an answer.

I'd like to use mypy to type check a recursive-descent programming language interpreter, and I am running into what seems like a pretty basic problem: defining the type for the expression tree. The textbook approach from, e.g., Haskell, is to define an algebraic data type of the possible syntactic forms in the language, which will presumably be recursive because the type of a general subexpression of something is "expression".

I was able to define the type in what seems to be the idiomatic mypy way, but when I tried to do isinstance case analysis on it, I got None types in the branches, even though the program runs. Am I doing something wrong?

Here is a small example of the kind of program I am trying to write:

from typing import List
from typing import NamedTuple
from typing import Union

Exp = Union['App', 'Lit']

# Make classes so that forward references work.
class App(NamedTuple('App', [('subs', List[Exp])])): pass
class Lit(NamedTuple('Lit', [('val', object)])): pass

def my_eval(exp):
  # type: (Exp) -> int
  reveal_type(exp)
  if isinstance(exp, App):
    reveal_type(exp)
    return sum([my_eval(e) for e in exp.subs])
  if isinstance(exp, Lit):
    reveal_type(exp)
    return exp.val

print(my_eval(App([Lit(1), Lit(2)])))

When run in Python 2.7 (removing the reveal_type, of course), this prints 3 as expected. Type checking in mypy, however, (both default and with --python-version 2.7) produces this output:

$ mypy test.py
test.py:13: error: Revealed type is 'Union[regen.App, regen.Lit]'
test.py:15: error: Revealed type is 'builtins.None'
test.py:16: error: None has no attribute "subs"
test.py:18: error: Revealed type is 'builtins.None'
test.py:19: error: None has no attribute "val"

Or, with --strict-optional, this:

$ mypy --strict-optional test.py
test.py:13: error: Revealed type is 'Union[regen.App, regen.Lit]'
test.py:15: error: Revealed type is '<uninhabited>'
test.py:16: error: object has no attribute "subs"
test.py:18: error: Revealed type is '<uninhabited>'
test.py:19: error: object has no attribute "val"

Is there a way to do this in mypy? Since this is a new project, I am willing to reorganize the (largely non-existent) code.

@ddfisher
Copy link
Collaborator

Playing around with this a bit, it looks to me like a bug in mypy. Here's the smallest case I reduced it to:

from typing import *

ForwardReferenceUnion = Union['SomeTuple', int]
class SomeTuple(NamedTuple('SomeTuple', [('elem', int)])): pass

def f(x: ForwardReferenceUnion) -> None:
  reveal_type(x)  # E: Revealed type is 'Union[test.SomeTuple, builtins.int]'
  if isinstance(x, SomeTuple):
    reveal_type(x)  # E: Revealed type is 'builtins.None'

The problem goes away if the Union is not a forward reference. Unfortunately, that's not an option in your case, due to the recursive definition of your datatypes. For now, I'd make Exp a class, and have App and Lit inherit from it. That should work pretty similarly to what you're trying to do here.

@ddfisher ddfisher added bug mypy got something wrong topic-named-tuple labels Jan 27, 2017
@ddfisher ddfisher changed the title How to do recursive algebraic data types in mypy? Forward references to NamedTuples don't behave correctly in some instances Jan 27, 2017
@axch
Copy link
Author

axch commented Jan 27, 2017

Thank you, that worked.

JukkaL pushed a commit that referenced this issue Sep 27, 2017
Forward references didn't work with anything apart from classes, for example 
this didn't work:

```
x: A
A = NamedTuple('A', [('x', int)])
```

The same situation was with `TypedDict`, `NewType`, and type aliases. The 
root problem is that these synthetic types are neither detected in first pass, 
nor fixed in third pass. In certain cases this can lead to crashes (first six issues 
below are various crash scenarios). This fixes these crashes by applying some 
additional patches after third pass.

Here is the summary of the PR:

* New simple wrapper type `ForwardRef` with only one field `link` is introduced 
  (with updates to type visitors)
* When an unknown type is found in second pass, the corresponding 
  `UnboundType` is wrapped in `ForwardRef`, it is given a "second chance" in 
  third pass.
* After third pass I record the "suspicious" nodes, where forward references and 
  synthetic types have been encountered and append patches (callbacks) to fix 
  them after third pass. Patches use the new visitor `TypeReplacer` (which is the 
  core of this PR).

Fixes #3340
Fixes #3419
Fixes #3674
Fixes #3685
Fixes #3799
Fixes #3836
Fixes #3881
Fixes #867
Fixes #2241
Fixes #2399
Fixes #1701
Fixes #3016
Fixes #3054
Fixes #2762
Fixes #3575
Fixes #3990
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-named-tuple
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants