FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-143504: Expose CELL status of a symbol in symtable by Yashp002 · Pull Request #143549 · python/cpython · GitHub

Repository navigation

gh-143504: Expose CELL status of a symbol in symtable - #143549

Merged
iritkatriel merged 27 commits into
python:mainfrom
Yashp002:symtable
Jan 25, 2026
Merged

iritkatriel merged 27 commits into
python:mainfrom
Yashp002:symtable

Conversation

Yashp002 commented Jan 8, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

Expose cell status in symtable.Symbol via new is_cell() predicate and add get_cells() method to SymbolTable.

Changes

  • Added Symbol.is_cell() method that returns True if the symbol has the CELL flag.
  • Added SymbolTable.get_cells() method that returns an iterable of cell variable names.
  • Added test_cells to Lib/test/test_symtable.py to verify the new API.

Tests

  • Added test_cells test using a closure pattern (outer -> inner accessing x).

Yashp002 changed the title bpo-143504: Expose CELL status of a symbol in symtable (GH-143504) gh-143504: Expose CELL status of a symbol in symtable (GH-143504) Jan 8, 2026
Comment thread Lib/symtable.py
Yashp002 and others added 2 commits January 8, 2026 16:17
Co-authored-by: AN Long <aisk@users.noreply.github.com>

Yashp002 commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

I'll fix this in a while, my apologies for quite a big fault in the code actually

Comment thread 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())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Let's put this test next to test_free.

Comment thread Lib/symtable.py
Comment thread 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()]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This probably belongs next to get_frees.

Copy link
Copy Markdown
Contributor Author

@iritkatriel I was forced to force push the symtable branch due to me updating the branch before for the CI.

Comment thread 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()]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why does this not follow the pattern of get_frees and get_nonlocals?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We'll have to use the _cells variable then, I'll add it.

Comment thread Lib/test/test_symtable.py Outdated
code="""def outer():
x=1
def inner():
return x"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

the other tests don't do all this. Can we follow the same pattern in this one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Unless there's a reason why we can't, then we should follow the same pattern. What's the difference between the cases?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

self.spam.lookup("x").is_cell()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

and get_cells test should be next to get_frees test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

and get_cells test should be next to get_frees test.

isn't test_cells already next to test_free?

Comment thread Lib/test/test_symtable.py Outdated
self.assertFalse(outer.lookup("inner").is_cell())
self.assertTrue(self.spam.lookup("x").is_cell())


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

get_cells is not tested at all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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",))

Comment thread Lib/symtable.py Outdated

picnixz commented Jan 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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

Comment thread Lib/symtable.py Outdated
Comment thread Lib/test/test_symtable.py Outdated
picnixz changed the title gh-143504: Expose CELL status of a symbol in symtable (GH-143504) gh-143504: Expose CELL status of a symbol in symtable Jan 23, 2026
Yashp002 and others added 3 commits January 24, 2026 13:24
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>
Comment thread Doc/library/symtable.rst Outdated

.. method:: get_cells()

Return a tuple containing the names of cell variables in this table.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

try formatting like the entry for get_frees with :term:

Yashp002 requested a review from AA-Turner as a code owner January 24, 2026 11:39
Comment thread Doc/library/symtable.rst Outdated
Comment thread Doc/whatsnew/3.15.rst Outdated
@@ -0,0 +1 @@
Add symtable.is_cell() and get_cells() methods for cell variable analysis.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are two News files now

Yashp002 and others added 4 commits January 24, 2026 20:56
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Comment thread Doc/library/symtable.rst
Return a tuple containing names of :term:`free (closure) variables <closure variable>`
in this function.

.. method:: get_cells()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please add the versionadded directive as well

Comment thread Doc/library/symtable.rst Outdated
@@ -0,0 +1,2 @@
Add :meth:`symtable.Function.get_cells` and :meth:`symtable.Symbol.is_cell` methods.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Remove the extra empty line

Yashp002 and others added 3 commits January 25, 2026 12:22
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Comment thread Lib/symtable.py Outdated
Comment thread Doc/library/symtable.rst Outdated
Yashp002 and others added 2 commits January 25, 2026 18:44
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Copy link
Copy Markdown
Member

Thank you @Yashp002

iritkatriel enabled auto-merge (squash) January 25, 2026 13:18

Copy link
Copy Markdown
Contributor Author

@iritkatriel , @picnixz Thanks for the help

iritkatriel merged commit a51bf70 into python:main Jan 25, 2026
83 of 87 checks passed
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL