Consume nested scope tables in optimized-out asserts by youknowone · Pull Request #7438 · RustPython/RustPython

youknowone · 2026-03-16T00:59:52Z

When -O flag removes assert statements, any nested scopes (generators, comprehensions, lambdas) inside the assert expression still have symbol tables in the sub_tables list. Without consuming them, the next_sub_table index gets misaligned, causing later scopes to use wrong symbol tables.

Walk the skipped assert expression with an AST visitor to find and consume nested scope symbol tables, keeping the index aligned with AST traversal order.

Summary by CodeRabbit

Bug Fixes
- Fixed scope-alignment and traversal issues during assertion optimization so nested scopes in complex expressions (lambdas, comprehensions, generators, defaults) are consumed in the correct order.
Tests
- Added tests covering optimized assertions with nested scopes and default values to ensure stable behavior.

coderabbitai · 2026-03-16T00:59:58Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 458ba444-6800-46ba-971c-1750ad4e2ae9

📥 Commits

Reviewing files that changed from the base of the PR and between 774803b and 7a2f43c.

⛔ Files ignored due to path filters (1)

Lib/test/_test_multiprocessing.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/codegen/src/compile.rs

📝 Walkthrough

Walkthrough

Adds logic to consume skipped nested symbol tables when optimized asserts are transformed, traversing expressions (lambdas, comprehensions, generators, etc.) to push/pop symbol tables so symbol-table state stays aligned with AST traversal.

Changes

Cohort / File(s)	Summary
Symbol table & optimized-assert handling `crates/codegen/src/compile.rs`	Adds `consume_next_sub_table` and `consume_skipped_nested_scopes_in_expr` methods, a `SkippedScopeVisitor`, and hooks to consume skipped nested scopes in the optimized-assert path to maintain scope-consumption order.
Tests (codegen) `crates/codegen/...tests/*`, `crates/codegen/...test_optimized_assert_preserves_nested_scope_order`	Adds/extends tests covering optimized-assert behavior with nested scopes and defaults to validate new scope-consumption logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Refactor compile_function #6001: Modifies compiler/codegen scope handling and symbol-table helpers; closely related to nested scope consumption and push/pop logic.
compiler enter_scope #5950: Refactors scope lifecycle (enter/exit scope) and symbol-table stack operations in compile.rs, related to this PR's scope-management changes.

Poem

🐇 I nibble through scopes, one by one,

Hopping where lambdas and loops are spun.
Tables pushed and popped in tidy rows,
No tangled nests where confusion grows.
A tidy compiler — hare happily done.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Consume nested scope tables in optimized-out asserts' directly describes the main change: handling symbol table consumption for nested scopes when assertions are optimized away.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

crates/codegen/src/compile.rs (1)

9252-9264: Nice regression test; consider adding two edge-case asserts.

Please also add optimized-assert cases where nested scopes appear in (1) lambda defaults and (2) the first comprehension iterator expression, since those are key cursor-order edge paths.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/compile.rs` around lines 9252 - 9264, Add two edge-case
variants to the optimized-assert nested-scope test: one where the nested-scope
appears inside a lambda default and one where it appears as the first iterator
expression of a comprehension. Update the test function
test_optimized_assert_preserves_nested_scope_order (which calls
compile_exec_optimized) to include code strings that mirror the existing test
but place the nested-list expression into a lambda default parameter (e.g. def
f(..., fn=lambda: <nested-scope> or default using a list comprehension result)
and another code string where the first comprehension iterator uses the
nested-scope expression (e.g. _formats = [self._types_mapping[type(item)] for
item in sequence]; then a comprehension like [x for x in (_formats_generator)
for y in ...] ), ensuring both are passed to compile_exec_optimized so the
optimizer’s cursor-order handling for lambda defaults and
first-comprehension-iterator paths is exercised.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/codegen/src/compile.rs`:
- Around line 7649-7656: The current match arm that immediately calls
self.consume_scope() for ast::Expr::Lambda and comprehension nodes skips
traversal of child expressions that are evaluated in the enclosing scope (e.g.,
lambda default argument expressions and the first iterator/iterable in
comprehensions), causing next_sub_table misalignment; change the handling so you
first explicitly walk the child expressions that are evaluated in the outer
scope (for Lambda: walk default values / kw_defaults and annotations; for
ListComp/SetComp/DictComp/Generator: walk the initial iterator/iterable and any
target/expression parts that belong to the outer scope) and only then call
self.consume_scope(), instead of unconditionally skipping child traversal or
calling ast::visitor::walk_expr(self, expr); update the match arms for
ast::Expr::Lambda and each comprehension variant in compile.rs to perform these
targeted walks and then consume_scope to preserve correct scope-consumption
ordering.

---

Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 9252-9264: Add two edge-case variants to the optimized-assert
nested-scope test: one where the nested-scope appears inside a lambda default
and one where it appears as the first iterator expression of a comprehension.
Update the test function test_optimized_assert_preserves_nested_scope_order
(which calls compile_exec_optimized) to include code strings that mirror the
existing test but place the nested-list expression into a lambda default
parameter (e.g. def f(..., fn=lambda: <nested-scope> or default using a list
comprehension result) and another code string where the first comprehension
iterator uses the nested-scope expression (e.g. _formats =
[self._types_mapping[type(item)] for item in sequence]; then a comprehension
like [x for x in (_formats_generator) for y in ...] ), ensuring both are passed
to compile_exec_optimized so the optimizer’s cursor-order handling for lambda
defaults and first-comprehension-iterator paths is exercised.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: fc469d55-ceed-4c2f-8c45-73f8d39213b9

📥 Commits

Reviewing files that changed from the base of the PR and between f49af3f and 774803b.

⛔ Files ignored due to path filters (1)

Lib/test/_test_multiprocessing.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/codegen/src/compile.rs

crates/codegen/src/compile.rs

When -O flag removes assert statements, any nested scopes (generators, comprehensions, lambdas) inside the assert expression still have symbol tables in the sub_tables list. Without consuming them, the next_sub_table index gets misaligned, causing later scopes to use wrong symbol tables. Walk the skipped assert expression with an AST visitor to find and consume nested scope symbol tables, keeping the index aligned with AST traversal order.

github-actions · 2026-03-16T03:34:51Z

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/types.py
[ ] test: cpython/Lib/test/test_types.py (TODO: 8)

dependencies:

types

dependent tests: (52 tests)

types: test_annotationlib test_ast test_asyncgen test_asyncio test_builtin test_call test_code test_collections test_compile test_compiler_assemble test_coroutines test_decorators test_descr test_dis test_doctest test_dtrace test_dynamicclassattribute test_email test_enum test_exception_group test_fstring test_funcattrs test_generators test_genericalias test_hmac test_importlib test_inspect test_listcomps test_marshal test_monitoring test_opcache test_os test_positional_only_arg test_pprint test_pyclbr test_pydoc test_raise test_string test_subclassinit test_subprocess test_tempfile test_threading test_trace test_traceback test_type_aliases test_type_annotations test_type_params test_types test_typing test_unittest test_xml_etree test_xml_etree_c

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

youknowone force-pushed the fix-optimizer branch from aa20ab0 to 774803b Compare March 16, 2026 01:52

youknowone marked this pull request as ready for review March 16, 2026 02:57

coderabbitai bot reviewed Mar 16, 2026

View reviewed changes

crates/codegen/src/compile.rs Show resolved Hide resolved

youknowone force-pushed the fix-optimizer branch from 774803b to 7a2f43c Compare March 16, 2026 03:34

youknowone merged commit f27490c into RustPython:main Mar 16, 2026
16 checks passed

youknowone deleted the fix-optimizer branch March 16, 2026 07:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consume nested scope tables in optimized-out asserts#7438

Consume nested scope tables in optimized-out asserts#7438
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:fix-optimizer

youknowone commented Mar 16, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

github-actions bot commented Mar 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

youknowone commented Mar 16, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 16, 2026

📦 Library Dependencies

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Mar 16, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 16, 2026 •

edited

Loading