-
Notifications
You must be signed in to change notification settings - Fork 2
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
evaluate namespaces recursively, and function namespaces, too #2
Conversation
Already found a bug using this, with a function body containing |
(The bug I found is python/cpython#104615) |
@@ -236,6 +236,7 @@ def statements(): | |||
st.from_type(ast.Global), | |||
st.from_type(ast.Expr), | |||
st.from_type(ast.FunctionDef), | |||
st.from_type(ast.Name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would just generate statements of the form a
, right? Not sure why that's useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the simplified reproducer for python/cpython#104602 a simple b
after the comprehension would expose the bug. (In your form it was a print(b)
but the print isn't necessary.) So it seems useful to allow these to be generated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. One concern is that this will just throw NameError in most cases, making the run tests less useful. But it can still help catch scoping bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although with only two allowed names, the chance of NameError
decreases some? You may be right, I think we will need to observe some actual runs.
This recursively finds functions and classes from the top-level namespace and evaluates both.
For functions we do this by inserting a
return locals()
as the last statement in every function, so we can call it to get its namespace.