compile_type_param_bound_or_default by youknowone · Pull Request #6005 · RustPython/RustPython
Summary by CodeRabbit
-
New Features
- Improved handling of type parameter bounds and default values by introducing explicit nested scopes for these expressions.
- Type parameter scopes inside classes now have enhanced access to class-level information.
-
Refactor
- Unified and streamlined the processing of type parameter bounds and defaults for better consistency and maintainability.
Walkthrough
This update refactors how type parameter bounds and defaults are compiled and scanned. It introduces dedicated methods and nested scopes for these expressions, ensuring type parameter bounds/defaults are processed in their own closure and symbol table context. Additionally, symbol tables now track whether type parameter scopes can access their parent class scope.
Changes
| File(s) | Change Summary |
|---|---|
compiler/codegen/src/compile.rs |
Added compile_type_param_bound_or_default method; refactored compile_type_params to use new helper and closures for bounds/defaults. |
compiler/codegen/src/symboltable.rs |
Added can_see_class_scope field to SymbolTable; added enter_type_param_block and scan_type_param_bound_or_default methods; updated scanning of type parameter bounds/defaults to use nested scopes. |
Sequence Diagram(s)
sequenceDiagram
participant Compiler
participant SymbolTable
participant Closure
Compiler->>SymbolTable: enter_type_param_block(name)
Note right of SymbolTable: Set can_see_class_scope, register __classdict__ if in class
Compiler->>Compiler: compile_type_param_bound_or_default(expr, name, allow_starred)
Compiler->>Closure: create closure from compiled code
Compiler->>Closure: call closure immediately
SymbolTable->>SymbolTable: leave type_param_block
Possibly related PRs
- RustPython/RustPython#6001: Refactors function/type parameter compilation using symbol tables and closures, closely related to this PR's modularization of type parameter handling.
- RustPython/RustPython#5909: Extends
compile_type_paramsfor default values and generic bases, overlapping with this PR's changes to type parameter compilation.
Poem
A warren of scopes, each nested with care,
Type bounds and defaults now breathe cleaner air.
With closures and tables, the logic’s refined—
Each bunny in code with its own space assigned!
🐇✨
Scope-hopping rabbits, compiling with glee,
Hopping through bounds, wild and free!
📜 Recent review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
compiler/codegen/src/compile.rs(5 hunks)compiler/codegen/src/symboltable.rs(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- compiler/codegen/src/compile.rs
- compiler/codegen/src/symboltable.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
✨ Finishing Touches
- 📝 Generate Docstrings
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.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Explain this complex logic.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai explain this code block.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and explain its main purpose.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
compiler/codegen/src/symboltable.rs (1)
1406-1420: Good refactoring to use nested scopes for type parameter expressions.The implementation correctly handles TypeVar bounds and defaults in separate scopes. The distinction between constraints (tuple expressions) and bounds is properly maintained.
Consider extracting the scope name generation into a helper method to reduce duplication:
+ fn type_param_scope_name(param_name: &str, scope_type: &str) -> String { + format!("<{} of {}>", scope_type, param_name) + }Then use it as:
- let scope_name = if binding.is_tuple_expr() { - format!("<TypeVar constraint of {}>", name.as_str()) - } else { - format!("<TypeVar bound of {}>", name.as_str()) - }; + let scope_type = if binding.is_tuple_expr() { + "TypeVar constraint" + } else { + "TypeVar bound" + }; + let scope_name = Self::type_param_scope_name(name.as_str(), scope_type);
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
compiler/codegen/src/compile.rs(5 hunks)compiler/codegen/src/symboltable.rs(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
🔇 Additional comments (11)
compiler/codegen/src/compile.rs (5)
1655-1696: LGTM! Well-structured implementation for type parameter scoping.The new method correctly implements the compilation of type parameter bounds and defaults in isolated scopes, which aligns with PEP 695 requirements. The implementation properly:
- Manages symbol table stack with
push_symbol_table()- Creates a nested
TypeParamsscope- Handles starred expressions for
TypeVarTupledefaults- Creates and immediately calls a closure to evaluate the expression
- Properly handles scope cleanup via
exit_scope()The comment on line 1687 correctly notes that
exit_scope()handles the symbol table cleanup, which is good documentation.
1710-1729: Correct application of scoped compilation for TypeVar bounds.The refactoring properly replaces direct
compile_expressioncalls with the newcompile_type_param_bound_or_defaultmethod. The logic correctly:
- Distinguishes between tuple expressions (constraints) and single bounds
- Uses appropriate scope names for debugging
- Maintains the existing intrinsic function selection logic
- Passes
allow_starred: falsewhich is correct for TypeVar bounds
1740-1742: Proper scoping for TypeVar defaults.Correctly applies the new compilation method for TypeVar default values with appropriate parameters (
allow_starred: falseis correct for TypeVar defaults).
1767-1769: Consistent application for ParamSpec defaults.Follows the same pattern as TypeVar defaults with correct parameters.
1794-1797: Correct handling of TypeVarTuple defaults with starred expressions.This usage correctly passes
allow_starred: true, which is the key distinction forTypeVarTupledefaults that can contain starred expressions (e.g.,*Ts). This is the only type parameter variant that allows this syntax.compiler/codegen/src/symboltable.rs (6)
58-59: LGTM! Well-documented field addition.The new
can_see_class_scopefield is properly documented and follows the existing struct field pattern.
74-74: LGTM! Appropriate default initialization.The field is correctly initialized to
false, which is the appropriate default for most scopes.
672-695: LGTM! Well-implemented type parameter scope handling.The method correctly:
- Detects class scope context before entering the new scope
- Sets
can_see_class_scopeflag appropriately- Registers necessary symbols (
__classdict__and.type_params) with proper usage types
1371-1389: LGTM! Clean implementation of nested scope for type parameter expressions.The method properly encapsulates the pattern of creating a nested scope for type parameter bounds and defaults, ensuring proper symbol resolution.
1428-1433: LGTM! Consistent handling of defaults for ParamSpec and TypeVarTuple.The implementation follows the same pattern as TypeVar, ensuring uniform behavior across all type parameter variants.
Also applies to: 1441-1446
778-781: LGTM! Consistent migration toenter_type_param_block.All three call sites (FunctionDef, ClassDef, and TypeAlias) have been correctly updated to use the new method while maintaining the same parameter structure.
Also applies to: 804-807, 994-997