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

Avoid infinite recursion in type queries #5434

Merged
merged 1 commit into from Aug 8, 2018

Conversation

Projects
None yet
2 participants
@ilevkivskyi
Collaborator

ilevkivskyi commented Aug 8, 2018

Fixes #5357
Fixes #4780

This might potentially fix similar crashes on recursive types.

@ilevkivskyi ilevkivskyi requested a review from msullivan Aug 8, 2018

@msullivan

Looks good!

@@ -1915,6 +1915,7 @@ class TypeQuery(SyntheticTypeVisitor[T]):
def __init__(self, strategy: Callable[[Iterable[T]], T]) -> None:
self.strategy = strategy
self.seen = [] # type: List[Type]

This comment has been minimized.

@msullivan

msullivan Aug 8, 2018

Collaborator

How big do we expect seen to get? If we think it will get pretty big we could make it a set of ids of Typess

@msullivan

msullivan Aug 8, 2018

Collaborator

How big do we expect seen to get? If we think it will get pretty big we could make it a set of ids of Typess

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Aug 8, 2018

Collaborator

My expectation is that seen should be never larger than few dozens of types. I think we discussed this with @JukkaL when implementing ForwardReferenceResolver that uses the same pattern and decided it is "small" (whatever it means).

@ilevkivskyi

ilevkivskyi Aug 8, 2018

Collaborator

My expectation is that seen should be never larger than few dozens of types. I think we discussed this with @JukkaL when implementing ForwardReferenceResolver that uses the same pattern and decided it is "small" (whatever it means).

This comment has been minimized.

@msullivan

msullivan Aug 8, 2018

Collaborator

👍

@msullivan

msullivan Aug 8, 2018

Collaborator

👍

@ilevkivskyi ilevkivskyi merged commit ca201f4 into python:master Aug 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:fix-recursive-crashes branch Aug 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment