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

Crash when serializing property with forward reference #4682

Closed
JukkaL opened this issue Mar 6, 2018 · 0 comments · Fixed by #4695
Closed

Crash when serializing property with forward reference #4682

JukkaL opened this issue Mar 6, 2018 · 0 comments · Fixed by #4695

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 6, 2018

This serialization test case fails with a crash:

[case testSerializeForwardReferenceToAliasInProperty]
import a
[file a.py]
import b
[file a.py.2]
import b
reveal_type(b.A().p)
[file b.py]

class A:
    @property
    def p(self) -> C: pass
    @p.setter
    def p(self, c: C) -> None: pass
    @p.deleter
    def p(self) -> None: pass

C = str
[builtins fixtures/property.pyi]

Here's the traceback:

/Users/jukka/src/mypy/mypy/test/testcheck.py:105: in run_case
    self.run_case_once(testcase, step)
/Users/jukka/src/mypy/mypy/test/testcheck.py:169: in run_case_once
    alt_lib_path=test_temp_dir)
/Users/jukka/src/mypy/mypy/build.py:180: in build
    result = _build(sources, options, alt_lib_path, bin_dir, saved_cache, flush_errors)
/Users/jukka/src/mypy/mypy/build.py:266: in _build
    graph = dispatch(sources, manager)
/Users/jukka/src/mypy/mypy/build.py:2114: in dispatch
    process_graph(graph, manager)
/Users/jukka/src/mypy/mypy/build.py:2421: in process_graph
    process_stale_scc(graph, scc, manager)
/Users/jukka/src/mypy/mypy/build.py:2621: in process_stale_scc
    graph[id].write_cache()
/Users/jukka/src/mypy/mypy/build.py:2071: in write_cache
    self.manager)
/Users/jukka/src/mypy/mypy/build.py:1260: in write_cache
    data = {'tree': tree.serialize(),
/Users/jukka/src/mypy/mypy/nodes.py:247: in serialize
    'names': self.names.serialize(self._fullname),
/Users/jukka/src/mypy/mypy/nodes.py:2521: in serialize
    data[key] = value.serialize(fullname, key)
/Users/jukka/src/mypy/mypy/nodes.py:2456: in serialize
    data['node'] = self.node.serialize()
/Users/jukka/src/mypy/mypy/nodes.py:2229: in serialize
    'names': self.names.serialize(self.fullname()),
/Users/jukka/src/mypy/mypy/nodes.py:2521: in serialize
    data[key] = value.serialize(fullname, key)
/Users/jukka/src/mypy/mypy/nodes.py:2458: in serialize
    data['type_override'] = self.type_override.serialize()
/Users/jukka/src/mypy/mypy/types.py:955: in serialize
    'items': [t.serialize() for t in self.items()],
/Users/jukka/src/mypy/mypy/types.py:955: in <listcomp>
    'items': [t.serialize() for t in self.items()],
/Users/jukka/src/mypy/mypy/types.py:870: in serialize
    'ret_type': self.ret_type.serialize(),
/Users/jukka/src/mypy/mypy/types.py:1429: in serialize
    assert False, "Internal error: Unresolved forward reference to {}".format(name)
E   AssertionError: Internal error: Unresolved forward reference to C

It looks like we don't remove forward references in the type_override attribute of symbol table nodes.

@JukkaL JukkaL self-assigned this Mar 6, 2018
JukkaL added a commit that referenced this issue Mar 8, 2018
This adds supports for some cases of importing an imported name
within an import cycle. Originally they could result in false positives
or false negatives.

The idea is to use a new node type ImportedName in semantic
analysis pass 1 to represent an indirect reference to a name in another
module. It will get resolved in semantic analysis pass 2.

ImportedName is not yet used everywhere where it could make
sense and this doesn't fix all related issues with import cycles.

Also did a bit of refactoring of type semantic analysis to avoid passing
multiple callback functions.

Fixes #4049.
Fixes #4429.
Fixes #4682.

Inspired by (and borrowed test cases from) #4495 by @carljm.

Supersedes #4495
yedpodtrzitko pushed a commit to kiwicom/mypy that referenced this issue Mar 15, 2018
This adds supports for some cases of importing an imported name
within an import cycle. Originally they could result in false positives
or false negatives.

The idea is to use a new node type ImportedName in semantic
analysis pass 1 to represent an indirect reference to a name in another
module. It will get resolved in semantic analysis pass 2.

ImportedName is not yet used everywhere where it could make
sense and this doesn't fix all related issues with import cycles.

Also did a bit of refactoring of type semantic analysis to avoid passing
multiple callback functions.

Fixes python#4049.
Fixes python#4429.
Fixes python#4682.

Inspired by (and borrowed test cases from) python#4495 by @carljm.

Supersedes python#4495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant