gh-143504: Expose CELL status of a symbol in symtable#143549
gh-143504: Expose CELL status of a symbol in symtable#143549iritkatriel merged 27 commits intopython:mainfrom
Conversation
Co-authored-by: AN Long <aisk@users.noreply.github.com>
|
I'll fix this in a while, my apologies for quite a big fault in the code actually |
Lib/test/test_symtable.py
Outdated
| 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()) |
There was a problem hiding this comment.
Let's put this test next to test_free.
Lib/symtable.py
Outdated
|
|
||
| def get_cells(self): | ||
| """Return a list of cell variable names in the table.""" | ||
| return [s.get_name() for s in self.get_symbols() if s.is_cell()] |
There was a problem hiding this comment.
This probably belongs next to get_frees.
|
@iritkatriel I was forced to force push the symtable branch due to me updating the branch before for the CI. |
Lib/symtable.py
Outdated
|
|
||
| def get_cells(self): | ||
| """Return a list of cell variable names in the table.""" | ||
| return [s.get_name() for s in self.get_symbols() if s.is_cell()] |
There was a problem hiding this comment.
Why does this not follow the pattern of get_frees and get_nonlocals?
There was a problem hiding this comment.
We'll have to use the _cells variable then, I'll add it.
Lib/test/test_symtable.py
Outdated
| code="""def outer(): | ||
| x=1 | ||
| def inner(): | ||
| return x""" |
There was a problem hiding this comment.
the other tests don't do all this. Can we follow the same pattern in this one?
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
Unless there's a reason why we can't, then we should follow the same pattern. What's the difference between the cases?
There was a problem hiding this comment.
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
top=symtable.symtable(code,"?","exec")
outer = find_block(top, "outer")
self.assertEqual(outer.get_cells(), ["x"])
self.assertTrue(outer.lookup("x").is_cell())
self.assertFalse(outer.lookup("inner").is_cell()) to use assert.Equal,true and false like the rest of the functions.
WOuld this be satisfactory as a commit?
There was a problem hiding this comment.
self.spam.lookup("x").is_cell()
There was a problem hiding this comment.
and get_cells test should be next to get_frees test.
There was a problem hiding this comment.
and get_cells test should be next to get_frees test.
isn't test_cells already next to test_free?
Lib/test/test_symtable.py
Outdated
| self.assertFalse(outer.lookup("inner").is_cell()) | ||
| self.assertTrue(self.spam.lookup("x").is_cell()) | ||
|
|
||
|
|
There was a problem hiding this comment.
get_cells is not tested at all.
There was a problem hiding this comment.
Add it to test_function_info:
@@ -255,6 +255,7 @@ def test_function_info(self):
self.assertEqual(sorted(func.get_locals()), expected)
self.assertEqual(sorted(func.get_globals()), ["bar", "glob", "some_assigned_global_var"])
self.assertEqual(self.internal.get_frees(), ("x",))
+ self.assertEqual(self.spam.get_cells(), ("some_var", "x",))
|
Can you also update symtable.rst? and add a What's New entry? (I did so for what I changed in 3.14) And once you update the docs, update the NEWS entry to point them to |
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Misc/NEWS.d/next/Library/2026-01-24-13-49-05.gh-issue-143594.nilGlg.rst
Outdated
Show resolved
Hide resolved
Doc/library/symtable.rst
Outdated
|
|
||
| .. method:: get_cells() | ||
|
|
||
| Return a tuple containing the names of cell variables in this table. |
There was a problem hiding this comment.
try formatting like the entry for get_frees with :term:
| @@ -0,0 +1 @@ | |||
| Add symtable.is_cell() and get_cells() methods for cell variable analysis. | |||
There was a problem hiding this comment.
There are two News files now
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
| Return a tuple containing names of :term:`free (closure) variables <closure variable>` | ||
| in this function. | ||
|
|
||
| .. method:: get_cells() |
There was a problem hiding this comment.
Please add the versionadded directive as well
| @@ -0,0 +1,2 @@ | |||
| Add :meth:`symtable.Function.get_cells` and :meth:`symtable.Symbol.is_cell` methods. | |||
|
|
|||
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
|
Thank you @Yashp002 |
|
@iritkatriel , @picnixz Thanks for the help |
Expose cell status in symtable.Symbol via new
is_cell()predicate and addget_cells()method to SymbolTable.Changes
Symbol.is_cell()method that returnsTrueif the symbol has the CELL flag.SymbolTable.get_cells()method that returns an iterable of cell variable names.test_cellstoLib/test/test_symtable.pyto verify the new API.Tests
test_cellstest using a closure pattern (outer -> inneraccessingx).