Skip to content
This repository has been archived by the owner. It is now read-only.

Do not leave List elements on the stack after visiting #852

Merged
merged 4 commits into from Jul 2, 2018

Conversation

@patiences
Copy link
Contributor

@patiences patiences commented Jun 27, 2018

When visiting a list node that's being stored into the list object should be popped from the stack when finished (this is already the case for Tuples in ast.py a few lines below my change). This fixes #850.

self.assertCodeExecution("""
try:
[x, y, z, a] = range(3)
except:
Copy link
Contributor Author

@patiences patiences Jun 27, 2018

If you are wondering why the exception is ignored, it's because VOC throws a StopIteration while Cpython throws a ValueError (this behavior is known and captured in SequenceTests and they are expected failures).

Copy link
Contributor Author

@patiences patiences Jun 28, 2018

Added the tests that catch the correct error in the next commit (as expected failures), so these can be removed when handling bad unpacking of sequences is implemented.

print(y)
print(z)
print(a)
except Exception as e:
Copy link
Member

@freakboy3742 freakboy3742 Jun 29, 2018

In retrospect, it's probably better to specifically catch StopIteration - that way the test will fail if/when we get around to fix the underlying problem, this test will fail, but hopefully the documentation will point at the way to make the test pass - to delete it :-)

Copy link
Contributor Author

@patiences patiences Jun 30, 2018

Yes it would be -- but then the test will fail because there will be no StopIteration to catch on the Python side. Or are you suggesting to use except (StopIteration, ValueError) as e:? I think that when this problem is fixed, the correct test below will result in an unexpected pass, and the comment there should be enough to point the human to delete this test?

Copy link
Member

@freakboy3742 freakboy3742 Jun 30, 2018

Sorry - yes, I meant to catch (StopIteration, ValueError) as e

Copy link
Contributor Author

@patiences patiences Jul 1, 2018

Done! :-)

@patiences patiences mentioned this pull request Jun 30, 2018
Copy link
Member

@freakboy3742 freakboy3742 left a comment

👍

@freakboy3742 freakboy3742 merged commit d7fa15d into beeware:master Jul 2, 2018
5 checks passed
@patiences patiences deleted the fix-list-bug branch Jul 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants