Fix symbol table sub_table desync for non-simple annotation targets#7300
Fix symbol table sub_table desync for non-simple annotation targets#7300youknowone merged 3 commits intoRustPython:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change narrows annotation scanning in the symbol table: annotation scopes are treated as non-async, and AnnAssign annotations are handled differently for simple names versus complex targets, validating complex annotations without creating separate annotation sub-tables. (48 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin fix-subtable |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/typing.py dependencies:
dependent tests: (17 tests)
Legend:
|
Non-simple annotations (subscript/attribute/parenthesized targets like `a[0]: expr`) were scanned in the annotation scope during symbol table analysis, creating sub_tables for any comprehensions. But codegen only compiles simple name annotations into __annotate__, so those sub_tables were never consumed. This caused subsequent simple annotations' comprehension sub_tables to get the wrong index, resulting in "the symbol 'X' must be present in the symbol table" errors. Fix: skip entering annotation scope for non-simple annotations since they are never compiled into __annotate__.
Fix cspell "desynchronize" warning and validate yield/await/named/async comprehension expressions in non-simple annotations without creating annotation scopes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/symboltable.rs`:
- Around line 1470-1484: The early-return in scan_expression when handling
non-simple annotation targets can bypass restoring the in_annotation flag;
update the branch in the function containing is_simple_name so that before
calling self.scan_expression(annotation, ExpressionContext::Load)? you set
was_in_annotation = self.in_annotation, set self.in_annotation = true, and
ensure self.in_annotation is restored to was_in_annotation even if
scan_expression returns an error (use a match/let res pattern or a
try/finally-style restore before propagating the error), mirroring the
restoration pattern used by scan_annotation; also add a regression test
exercising a non-simple annotated assignment with a forbidden expression (e.g.,
x[0]: (yield 1) = 1) to the test suite to verify the flag is always restored.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_type_annotations.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/codegen/src/symboltable.rs
Non-simple annotations (subscript/attribute/parenthesized targets like
a[0]: expr) were scanned in the annotation scope during symbol table analysis, creating sub_tables for any comprehensions. But codegen only compiles simple name annotations into annotate, so those sub_tables were never consumed. This caused subsequent simple annotations' comprehension sub_tables to get the wrong index, resulting in "the symbol 'X' must be present in the symbol table" errors.Fix: skip entering annotation scope for non-simple annotations since they are never compiled into annotate.
Summary by CodeRabbit