Fix sub_table ordering for nested inlined comprehensions (PEP 709)#7480
Conversation
When an inlined comprehension's first iterator expression contains
nested scopes (such as a lambda), those scopes' sub_tables appear at the
current position in the parent's sub_table list. The previous code
spliced the comprehension's own child sub_tables (e.g. inner inlined
comprehensions) into that same position before compiling the iterator,
which shifted the iterator's sub_tables to wrong indices.
Move the splice after the first iterator is compiled so its sub_tables
are consumed at their original positions.
Fixes nested list comprehensions like:
```python
[[x for _, x in g] for _, g in itertools.groupby(..., lambda x: ...)]
```
Disclosure: I used AI to develop the patch though I was heavily
involved.
📝 WalkthroughWalkthroughReorders compilation sequence in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
extra_tests/snippets/syntax_comprehension.py (1)
50-81: Ensure this snippet suite is executed in debug mode in CI/test docs.These additions are in
extra_tests/snippets/**; keeping execution oncargo run(debug) avoids unnecessary compile-time overhead for this suite.As per coding guidelines, "
extra_tests/snippets/**: Use debug mode (cargo run) forextra_tests/snippetstests as compilation is faster".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extra_tests/snippets/syntax_comprehension.py` around lines 50 - 81, Update CI/test documentation and the test-runner config so the extra_tests/snippets suite runs in debug mode (via cargo run) instead of a release/compiled invocation; specifically ensure any CI job or test script that executes tests under the directory extra_tests/snippets uses cargo run (debug) to invoke these snippet files (including test_nested_comp_with_lambda, test_nested_comp_underscore, test_simple_nested_comp) to avoid unnecessary compile-time overhead. Adjust the relevant CI step or README/test docs to call cargo run for that path and confirm the change is applied where extra_tests/snippets is referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@extra_tests/snippets/syntax_comprehension.py`:
- Around line 50-81: Update CI/test documentation and the test-runner config so
the extra_tests/snippets suite runs in debug mode (via cargo run) instead of a
release/compiled invocation; specifically ensure any CI job or test script that
executes tests under the directory extra_tests/snippets uses cargo run (debug)
to invoke these snippet files (including test_nested_comp_with_lambda,
test_nested_comp_underscore, test_simple_nested_comp) to avoid unnecessary
compile-time overhead. Adjust the relevant CI step or README/test docs to call
cargo run for that path and confirm the change is applied where
extra_tests/snippets is referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 51cda9eb-baf9-4d88-bc93-6fa42c52ff77
📒 Files selected for processing (2)
crates/codegen/src/compile.rsextra_tests/snippets/syntax_comprehension.py
youknowone
left a comment
There was a problem hiding this comment.
Thank you for contributing! You don't need to worry about AI develop as long as you are in the loop.
When an inlined comprehension's first iterator expression contains nested scopes (such as a lambda), those scopes' sub_tables appear at the current position in the parent's sub_table list. The previous code spliced the comprehension's own child sub_tables (e.g. inner inlined comprehensions) into that same position before compiling the iterator, which shifted the iterator's sub_tables to wrong indices.
Move the splice after the first iterator is compiled so its sub_tables are consumed at their original positions.
Fixes nested list comprehensions like:
Disclosure: I used AI to develop the patch though I was heavily involved.
Summary by CodeRabbit
Bug Fixes