GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix symtable for named_expressions by youknowone · Pull Request #6604 · RustPython/RustPython

Conversation

@youknowone

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
youknowone added 3 commits December 31, 2025 12:34
@youknowone
check duplicated type parameter
03cf6e6
@youknowone
typealias
93abd3b
@youknowone
fix self.in_comp_inner_loop_target
767c3e6
@coderabbitai

coderabbitai Bot commented Dec 31, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
📝 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

  • compile_type_param_bound_or_default #6005: Modifies scan_type_param_bound_or_default() and type-parameter-scoping logic, directly intersecting with this PR's type-parameter validation enhancements.
  • Remove misplaced SymbolScope::TypeParams #5975: Modifies symbol table handling for type-parameter scopes and introduces context-aware error checks similar to this PR's approach.
  • Refactor compile_function #6001: Modifies symbol table expression-scanning to restrict named expressions in type parameter scopes, aligning with this PR's validation goals.

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands and usage tips.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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

Uh oh!

There was an error while loading. Please reload this page.
@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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@youknowone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.