Skip to content

Conversation

@Yashp002
Copy link
Contributor

@Yashp002 Yashp002 commented Jan 8, 2026

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 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
Yashp002 and others added 2 commits January 8, 2026 16:17
Co-authored-by: AN Long <aisk@users.noreply.github.com>
@Yashp002
Copy link
Contributor Author

Yashp002 commented Jan 8, 2026

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

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
Member

Choose a reason for hiding this comment

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

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()]
Copy link
Member

Choose a reason for hiding this comment

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

This probably belongs next to get_frees.

@Yashp002
Copy link
Contributor Author

@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()]
Copy link
Member

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?

Copy link
Contributor Author

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.

code="""def outer():
x=1
def inner():
return x"""
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

    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
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

and get_cells test should be next to get_frees test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and get_cells test should be next to get_frees test.

isn't test_cells already next to test_free?

self.assertFalse(outer.lookup("inner").is_cell())
self.assertTrue(self.spam.lookup("x").is_cell())


Copy link
Member

Choose a reason for hiding this comment

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

get_cells is not tested at all.

Copy link
Member

Choose a reason for hiding this comment

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

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

"""Return a list of cell variable names in the table.
"""
if self.__cells is None:
self.__cells = [s.get_name() for s in self.get_symbols() if s.is_cell()]
Copy link
Member

Choose a reason for hiding this comment

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

Let's follow the pattern that it used in get_frees:

Suggested change
self.__cells = [s.get_name() for s in self.get_symbols() if s.is_cell()]
is_cell = lambda x: _get_scope(x) == CELL
self.__cells = self.__idents_matching(is_cell)

@picnixz
Copy link
Member

picnixz commented Jan 23, 2026

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

return self.__frees

def get_cells(self):
"""Return a list of cell variable names in the table.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Return a list of cell variable names in the table.
"""Return a tuple of cell variable names in the table.

def test_cells(self):
self.assertTrue(self.spam.lookup("x").is_cell())


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

There ate 2 blanks instead of 1 here.

@picnixz 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants