Remove misplaced SymbolScope::TypeParams #5975
Conversation
WalkthroughThe changes systematically replace the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Compiler
participant SymbolTable
User->>Compiler: enter_scope(name, scope_type, key, lineno)
Compiler->>SymbolTable: enter_scope(name, CompilerScope, lineno)
SymbolTable-->>Compiler: Scope entered (with new enum)
Compiler-->>User: Compilation continues
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
compiler/codegen/src/compile.rs(10 hunks)compiler/codegen/src/symboltable.rs(22 hunks)vm/src/stdlib/symtable.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
🧠 Learnings (2)
vm/src/stdlib/symtable.rs (1)
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T10:08:48.858Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
compiler/codegen/src/compile.rs (1)
undefined
<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T10:08:48.858Z
Learning: Applies to **/*.rs : Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust
</retrieved_learning>
🔇 Additional comments (18)
vm/src/stdlib/symtable.rs (1)
8-10: Import update looks correct
CompilerScopeis now re-exported fromrustpython_codegen::symboltable; the path and naming are accurate.compiler/codegen/src/compile.rs (6)
412-412: LGTM: Function parameter type correctly updatedThe function parameter type change from
SymbolTableTypetoCompilerScopeis correct and aligns with the systematic enum replacement throughout the codebase.
455-455: LGTM: Debug assertions correctly updatedThe debug assertions are properly updated to use
CompilerScope::Classinstead of the old enum variant, maintaining the same validation logic.Also applies to: 462-462
482-502: LGTM: Match arms comprehensively updatedThe match arms are systematically updated to use
CompilerScopevariants. The logic correctly handles all scope types including the newAsyncFunctionvariant and theTypeParamsvariant (which is now properly scoped as a compiler scope rather than a symbol scope).
533-533: LGTM: Scope type comparisons correctly updatedAll comparisons with scope types are properly updated to use
CompilerScopevariants, maintaining the same conditional logic with the new enum type.Also applies to: 547-547, 552-552, 572-572
953-953: LGTM: Commented code appropriately documents removed variantThe commented line indicates that
SymbolScope::Unknownwas removed as part of this refactoring, which is appropriate since it's no longer needed in the current implementation.
2203-2203: LGTM: Function call correctly updatedThe call to
enter_scopeis properly updated to passCompilerScope::Classinstead of the old enum variant, maintaining the same functionality.compiler/codegen/src/symboltable.rs (11)
34-34: LGTM: Enum type correctly updated.The type annotation has been properly updated from
SymbolTableTypetoCompilerScope.
60-60: LGTM: Function parameter type correctly updated.The parameter type has been properly updated to use
CompilerScope.
91-99: LGTM: Enum renaming and extension looks good.The enum has been properly renamed from
SymbolTableTypetoCompilerScopewith the addition of theAsyncFunctionvariant. The variant naming is consistent with the existing pattern.
101-119: LGTM: Display implementation correctly updated.The
Displaytrait implementation has been properly updated to handle the new enum name and includes the newAsyncFunctionvariant with appropriate formatting.
537-537: LGTM: Comprehensive async function handling.The match arm correctly includes all function-like scopes including the new
AsyncFunctionvariant alongsideFunctionandLambda. This ensures consistent handling of async functions in symbol analysis.
641-641: LGTM: Module scope initialization correctly updated.The top-level scope initialization properly uses the new
CompilerScope::Moduleenum reference.
656-656: LGTM: Function signature correctly updated.The function parameter type has been properly updated to use
CompilerScope.
660-660: LGTM: Enum reference correctly updated.The comparison properly uses the new
CompilerScope::Functionenum reference.
751-751: LGTM: Type parameter scope references correctly updated.All references to
CompilerScope::TypeParamsare properly updated and consistent throughout the file.Also applies to: 779-779, 971-971
1456-1456: LGTM: Function scope creation correctly updated.The function scope creation properly uses the new
CompilerScope::Functionenum reference.
326-326: LGTM: All remaining enum references correctly updated.All remaining references to the enum have been systematically and correctly updated from
SymbolTableTypetoCompilerScopethroughout the file. The changes are consistent and maintain the correct functionality.Also applies to: 352-352, 362-362, 368-368, 432-433, 455-455, 480-480, 485-485, 526-526, 529-529, 555-555, 586-586, 1173-1173, 1193-1193, 1259-1259, 1290-1290
| #[pymethod] | ||
| fn is_optimized(&self) -> bool { | ||
| self.symtable.typ == SymbolTableType::Function | ||
| self.symtable.typ == CompilerScope::Function | ||
| } |
There was a problem hiding this comment.
is_optimized() ignores AsyncFunction scopes
With CompilerScope now containing an AsyncFunction variant, the optimisation check should treat both Function and AsyncFunction the same way (mirroring CPython, where async defs share the function-block semantics).
- fn is_optimized(&self) -> bool {
- self.symtable.typ == CompilerScope::Function
+ fn is_optimized(&self) -> bool {
+ matches!(
+ self.symtable.typ,
+ CompilerScope::Function | CompilerScope::AsyncFunction
+ )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[pymethod] | |
| fn is_optimized(&self) -> bool { | |
| self.symtable.typ == SymbolTableType::Function | |
| self.symtable.typ == CompilerScope::Function | |
| } | |
| #[pymethod] | |
| fn is_optimized(&self) -> bool { | |
| matches!( | |
| self.symtable.typ, | |
| CompilerScope::Function | CompilerScope::AsyncFunction | |
| ) | |
| } |
🤖 Prompt for AI Agents
In vm/src/stdlib/symtable.rs around lines 71 to 74, the is_optimized() method
currently only checks if self.symtable.typ equals CompilerScope::Function,
ignoring the AsyncFunction variant. Update the method to return true if
self.symtable.typ is either CompilerScope::Function or
CompilerScope::AsyncFunction, ensuring both are treated equivalently for
optimization checks.
Summary by CodeRabbit
New Features
Refactor