-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-143504: Expose CELL status of a symbol in symtable #143549
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
Changes from 8 commits
b840e4e
0b676e4
2175829
17072bd
f096488
57b36be
5c558f4
19cd54d
a9e5bf7
f437b13
54c8de1
3732812
d36cd84
a1ebb69
611d4d3
700a24f
2b96cb5
8c3a597
b36df01
2539357
b8df659
68653ee
6a6da41
b5e8482
14a6c28
a8bb25a
43b3cfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -284,6 +284,20 @@ def test_local(self): | |
| def test_free(self): | ||
| self.assertTrue(self.internal.lookup("x").is_free()) | ||
|
|
||
| def test_cells(self): | ||
| #test for addition of is_cell() and get_cells() | ||
| #see https://github.com/python/cpython/issues/143504 | ||
| code="""def outer(): | ||
| x=1 | ||
| def inner(): | ||
| return x""" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the other tests don't do all this. Can we follow the same pattern in this one?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @iritkatriel I've implemented the change in symtable.py with get_cells But the current test_cells seems to be the only one that keeps working after I've change it up quite a few times. Is it really a requirement for it all to have the same pattern?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless there's a reason why we can't, then we should follow the same pattern. What's the difference between the cases?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. self.internal has x as FREE var (for test_free). we need x as CELL var (outer scope). setUp doesn't have that so inline code is needed. However I've removed the unnecessary comments and changed the rest of it to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. self.spam.lookup("x").is_cell()
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and get_cells test should be next to get_frees test.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
isn't test_cells already next to test_free? |
||
|
|
||
| top=symtable.symtable(code,"?","exec") | ||
| outer = find_block(top, "outer") | ||
| self.assertIn("x",outer.get_cells()) | ||
| self.assertTrue(outer.lookup("x").is_cell()) | ||
| self.assertFalse(outer.lookup("inner").is_cell()) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. get_cells is not tested at all.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add it to test_function_info:
Yashp002 marked this conversation as resolved.
Outdated
|
||
| def test_referenced(self): | ||
| self.assertTrue(self.internal.lookup("x").is_referenced()) | ||
| self.assertTrue(self.spam.lookup("internal").is_referenced()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add symtable.is_cell() and get_cells() methods for cell variable analysis. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two News files now |
||
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.
Why does this not follow the pattern of get_frees and get_nonlocals?
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.
We'll have to use the _cells variable then, I'll add it.