Skip to content

Fix symbol table sub_table desync for non-simple annotation targets#7300

Merged
youknowone merged 3 commits intoRustPython:mainfrom
youknowone:fix-subtable
Mar 2, 2026
Merged

Fix symbol table sub_table desync for non-simple annotation targets#7300
youknowone merged 3 commits intoRustPython:mainfrom
youknowone:fix-subtable

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Mar 1, 2026

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

  • Refactor
    • Adjusted how annotation scopes are treated so annotations are always handled as non-async.
    • Improved annotation validation for assignment targets: retained detailed scanning for simple names while using safer validation for complex targets to avoid inconsistent internal annotation state.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b61e74 and 8bd6e97.

📒 Files selected for processing (1)
  • crates/codegen/src/symboltable.rs

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Symbol Table Annotation Handling
crates/codegen/src/symboltable.rs
Short-circuit is_in_async_context to non-async when in_annotation is true. In AnnAssign, simple-name targets use scan_annotation(annotation); non-simple targets skip direct annotation scanning, temporarily set in_annotation = true, validate via scan_expression(annotation, Load), then restore state to avoid creating mismatched annotation sub-tables.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through scopes with careful feet,
Marked annotations tidy, neat,
Simple names sing their old refrain,
Complex ones get checked, then hop again,
A little rabbit cheers the table's feat!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix symbol table sub_table desync for non-simple annotation targets' directly addresses the main problem described in the PR objectives: preventing sub_table creation for non-simple annotation targets to fix index desynchronization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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

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.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

Code has been automatically formatted

The code in this PR has been formatted using:

  • cargo fmt --all
    Please pull the latest changes before pushing again:
git pull origin fix-subtable

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

📦 Library Dependencies

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

[x] lib: cpython/Lib/typing.py
[ ] test: cpython/Lib/test/test_typing.py (TODO: 4)
[x] test: cpython/Lib/test/test_type_aliases.py
[x] test: cpython/Lib/test/test_type_annotations.py
[x] test: cpython/Lib/test/test_type_params.py (TODO: 6)
[x] test: cpython/Lib/test/test_genericalias.py

dependencies:

  • typing

dependent tests: (17 tests)

  • typing: test_annotationlib test_builtin test_copy test_enum test_fractions test_funcattrs test_functools test_genericalias test_grammar test_inspect test_isinstance test_patma test_pydoc test_type_aliases test_type_params test_types test_typing

Legend:

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

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.
@youknowone youknowone marked this pull request as ready for review March 2, 2026 03:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 888e0ee and 8b61e74.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_type_annotations.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/codegen/src/symboltable.rs

@youknowone youknowone merged commit baba1f9 into RustPython:main Mar 2, 2026
13 of 14 checks passed
@youknowone youknowone deleted the fix-subtable branch March 2, 2026 07:47
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.

1 participant