SymbolTableType::Lambda #5942
SymbolTableType::Lambda #5942
Conversation
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VM
participant FunctionObject
User->>VM: Call intrinsic SetFunctionTypeParams(func, type_params)
VM->>FunctionObject: set_attr("__type_params__", type_params)
FunctionObject-->>VM: Attribute set
VM-->>User: Return updated function object
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: 0
🧹 Nitpick comments (1)
compiler/core/src/bytecode.rs (1)
414-416: Minor doc / typo cleanup
// PrepReraiseS tar = 1,contains an extraneous space after theS. While unrelated to functionality, it’s a readability papercut.- // PrepReraiseS tar = 1, + // PrepReraiseStar = 1,
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
compiler/codegen/src/symboltable.rs(4 hunks)compiler/core/src/bytecode.rs(1 hunks)vm/src/frame.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Follow the default rustfmt code style (`cargo fmt` to format) Always ...
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your 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
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
vm/src/frame.rscompiler/core/src/bytecode.rscompiler/codegen/src/symboltable.rs
🧠 Learnings (1)
compiler/codegen/src/symboltable.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
⏰ 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). (11)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run tests under miri
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
🔇 Additional comments (6)
compiler/core/src/bytecode.rs (1)
414-420: Confirm marshal/version bump – new discriminantSetFunctionTypeParams = 4enters the wire formatEnabling this variant introduces a previously-unused tag (
4) in theIntrinsicFunction2enum that is marshalled as a rawu8.If any compiled bytecode or persisted snapshot created before this PR already emitted the value
4for a different meaning (e.g. due to the gaps at 1-3 being skipped), deserialization will now mis-interpret that byte and crash or misbehave.
Please double-check:
- The bytecode‐marshal version header (
MARSHAL_VERSIONor equivalent) is bumped, or- We are certain no historical artefacts can contain the tag
4, and- Integration tests exercising (de)serialisation are updated.
If uncertainty exists, bumping the version is the safer route.
vm/src/frame.rs (1)
2289-2294: LGTM: Clean implementation of SetFunctionTypeParams intrinsic function.The implementation correctly handles setting the
__type_params__attribute on function objects. The approach follows the established pattern for intrinsic functions and appropriately relies on the compiler to generate correct bytecode with proper argument types.compiler/codegen/src/symboltable.rs (4)
83-83: LGTM! Well-structured enum variant addition.The addition of
Lambdato theSymbolTableTypeenum is clean and follows the existing pattern. This enables proper distinction between lambda expressions and regular functions in the symbol table hierarchy.
94-94: LGTM! Consistent Display implementation.The Display trait implementation for the new
Lambdavariant follows the established pattern and uses appropriate lowercase naming consistent with other variants.
498-498: LGTM! Logical grouping of function-like scopes.The match arm correctly groups
SymbolTableType::FunctionandSymbolTableType::Lambdatogether, as they should be handled identically in comprehension analysis. This maintains the existing behavior while supporting the new lambda scope type.
1145-1145: LGTM! Proper lambda scope differentiation.The change from
SymbolTableType::FunctiontoSymbolTableType::Lambdawhen entering lambda scopes enables proper differentiation between lambda expressions and regular functions in the symbol table, which aligns with the PR objectives.
Summary by CodeRabbit
New Features
Improvements