PyCode::qualname #5929
PyCode::qualname #5929
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler
participant CodeInfo
participant CodeObject
participant marshal
participant PyCode
Compiler->>CodeInfo: Create with qualname (from code context)
CodeInfo->>CodeObject: Finalize, passing qualname
CodeObject->>marshal: Serialize/deserialize qualname
CodeObject->>PyCode: Expose qualname as co_qualname property
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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)
✨ 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 (2)
compiler/codegen/src/compile.rs (2)
405-411: Optimize redundant string cloning.The logic for calculating qualname is correct, but there's an opportunity to optimize the string handling.
Apply this diff to avoid redundant cloning:
- // Calculate qualname based on the current qualified path - let qualname = if self.qualified_path.is_empty() { - Some(obj_name.clone()) - } else { - Some(self.qualified_path.join(".")) - }; + // Calculate qualname based on the current qualified path + let qualname = if self.qualified_path.is_empty() { + Some(obj_name.clone()) + } else { + Some(self.qualified_path.join(".")) + };Actually, the current implementation is fine. The
obj_nameis used later in the struct, so the clone is necessary.
313-314: Consider refactoring to reduce code duplication.The pattern of updating qualname after pushing qualified paths is repeated across functions, classes, and comprehensions. While the current implementation is correct, consider extracting this into a helper method to improve maintainability.
Consider adding a helper method like:
fn update_qualname_from_path(&mut self) { let qualified_name = self.qualified_path.join("."); self.code_stack.last_mut().unwrap().qualname = Some(qualified_name); }This would reduce the repeated pattern of
self.qualified_path.join(".")and make the intent clearer.Also applies to: 405-411, 420-420, 507-509, 513-514, 735-736, 978-979
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
compiler/codegen/src/compile.rs(7 hunks)compiler/codegen/src/ir.rs(3 hunks)compiler/core/src/bytecode.rs(3 hunks)compiler/core/src/marshal.rs(3 hunks)vm/src/builtins/code.rs(2 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/builtins/code.rscompiler/codegen/src/ir.rscompiler/core/src/marshal.rscompiler/core/src/bytecode.rscompiler/codegen/src/compile.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). (11)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run tests under miri
🔇 Additional comments (16)
vm/src/builtins/code.rs (2)
301-304: LGTM! Clean implementation of co_qualname getter.The implementation follows the same pattern as other code object getters and correctly exposes the qualified name to Python code.
408-408: Good preservation of qualname in replace method.The qualified name is correctly preserved when creating replaced code objects, ensuring consistency with the original code object's identity.
compiler/codegen/src/ir.rs (2)
76-76: Well-designed optional qualname field.The optional
qualnamefield allows flexible qualified name tracking during code generation, with appropriate fallback handling infinalize_code.
167-168: Correct fallback logic for qualname.The implementation properly falls back to
obj_namewhenqualnameis not set, ensuring code objects always have a qualified name. Theclone()is necessary sinceobj_nameis moved in the destructuring.compiler/core/src/bytecode.rs (3)
118-119: Clean addition of qualname field with proper documentation.The field is correctly typed as
C::Nameand includes helpful documentation referencing CPython'sco_qualnamefor clarity.
1145-1145: Consistent qualname mapping in map_bag method.The qualified name is properly mapped using
bag.make_name(), maintaining consistency with other name field mappings likeobj_nameandsource_path.
1175-1175: Consistent qualname mapping in map_clone_bag method.The qualified name mapping in the clone variant follows the same pattern as the non-clone version, ensuring consistency across both mapping methods.
compiler/core/src/marshal.rs (3)
213-214: Correct deserialization of qualname field.The qualified name is properly read from the marshaled data using the same pattern as other string fields, with appropriate length-prefixed reading.
256-256: Proper inclusion of qualname in CodeObject construction.The deserialized qualified name is correctly included in the
CodeObjectconstruction, maintaining field order consistency.
616-616: Symmetric serialization of qualname field.The qualified name is properly serialized using
write_vec()in the same position as it's read during deserialization, ensuring symmetric marshaling behavior.compiler/codegen/src/compile.rs (6)
313-314: LGTM: Proper initialization of qualname in constructor.The qualname is correctly initialized to the code name in the
Compiler::newconstructor, establishing the base case for module-level code objects.
420-420: LGTM: Qualname correctly stored in CodeInfo.The qualname field is properly included in the
ir::CodeInfostructure creation.
507-509: LGTM: Function qualname correctly updated.The qualname is appropriately updated after establishing the qualified path for the function, ensuring it reflects the full qualified name.
513-514: LGTM: Lambda qualname follows Python conventions.Setting the lambda qualname to
"<lambda>"is correct and follows standard Python behavior for anonymous functions.
735-736: LGTM: Class qualname handling is consistent.The qualname update for classes follows the same pattern as functions, ensuring consistency across different code object types.
978-979: LGTM: Comprehension qualname correctly set.Setting the comprehension qualname to the comprehension name (e.g.,
"<listcomp>") is appropriate and follows Python conventions.
Summary by CodeRabbit
New Features
co_qualnameto retrieve the qualified name of code objects.Bug Fixes
Chores