Skip to content

fix symtable for named_expressions #6604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
youknowone merged 3 commits into from
Dec 31, 2025
Merged

fix symtable for named_expressions #6604

youknowone merged 3 commits into from
Dec 31, 2025

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 31, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error detection and reporting in type parameter definitions, type aliases, and comprehensions with contextual error messages and precise source locations for improved debugging.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The changes enhance the symbol table builder with context-aware error checking. New contextual state flags track whether expressions are processed within type parameters, annotations, type aliases, or inner comprehension loops. Error messages now reflect the precise context, and new validation logic prohibits named expressions in type parameters and detects inner-loop comprehension rebindings.

Changes

Cohort / File(s) Summary
Symbol table context and validation
crates/codegen/src/symboltable.rs		 Added contextual state fields (in_annotation, in_type_alias, in_comp_inner_loop_target, scope_info) to track expression processing context. Enhanced register_name() to enforce INNER_LOOP_CONFLICT checks for comprehension rebindings. Modified scan_type_param_bound_or_default() signature to accept scope_name and scope_info for richer error messages. Updated scan_comprehension() to toggle in_comp_inner_loop_target for inner generators. Extended scan_type_params() to detect duplicate type parameter names. Enhanced TypeVar, ParamSpec, TypeVarTuple, and TypeAlias handling to pass context information and track in_type_alias state. Introduced dedicated error messages indicating exact context (e.g., "within a type parameter", "in a TypeVar bound").

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A rabbit checks each context with care,
Named expressions forbidden in type-param air,
Comprehensions rebind? Not here, my friend!
With scope-aware errors, the mistakes we'll mend,
From bounds to defaults, precision's our way,
The symbol table grows wiser today!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 directly describes the main change: fixing symbol table handling for named expressions (walrus operator) with context-aware error detection and reporting.
✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 37c47fc and 767c3e6.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_named_expressions.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/codegen/src/symboltable.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/codegen/src/symboltable.rs
🧬 Code graph analysis (1)
crates/codegen/src/symboltable.rs (1)
crates/codegen/src/compile.rs (5)
  • new (338-345)
  • new (366-408)
  • name (909-911)
  • default (128-133)
  • default (332-334)
🔇 Additional comments (10)
crates/codegen/src/symboltable.rs (10)

588-592: LGTM!

The new error path correctly handles the case where a named expression in a comprehension is encountered within a type parameter scope. The error message aligns with CPython's behavior.

624-631: LGTM!

The new state tracking fields are well-documented and appropriately typed. Using &'static str for scope_info is efficient for the string literal error message contexts.

770-774: LGTM!

The save-and-restore pattern correctly handles nested annotations and properly restores the previous state after scanning.

1043-1056: LGTM!

The in_type_alias flag is correctly scoped to the value expression only, not the name being defined. The save-and-restore pattern properly handles nested type aliases.

1093-1126: LGTM!

The context-aware validation logic correctly prioritizes specific scope info over general context flags. The error messages are clear and include source locations for better diagnostics.

1472-1477: LGTM!

The inner loop target tracking correctly identifies non-first generators in comprehensions.

Note: If scan_expression returns an error, in_comp_inner_loop_target won't be reset, but since errors propagate up and terminate symbol table building, this is acceptable.

1501-1527: LGTM!

The function properly manages scope lifecycle with cleanup happening before returning the result, ensuring scope_info is restored and leave_scope is called even when scan_expression fails.

1530-1548: LGTM!

The duplicate type parameter detection is correctly implemented with clear error messages and proper source locations.

1567-1621: LGTM!

The scope naming and context info are well-structured, correctly distinguishing between TypeVar bounds vs. constraints based on tuple expressions, and providing meaningful scope names for each type parameter variant.

1760-1775: LGTM!

The inner loop conflict check correctly implements PEP 572's restriction, preventing comprehension inner loops from rebinding variables that were previously used as named expression targets.

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.

@youknowone youknowone marked this pull request as ready for review December 31, 2025 05:58
@youknowone youknowone merged commit b3b97ca into RustPython:main Dec 31, 2025
13 checks passed
@youknowone youknowone deleted the symtable branch December 31, 2025 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone