Remove Copy from PyPayload#6865
Conversation
This reverts commit 2f76ef6.
📝 WalkthroughWalkthroughThis PR removes Clone and Copy derives from numerous structs and enums across the VM codebase, alongside removing a workspace lint setting, to reduce automatic copy semantics on public types while adding new module creation methods to PyModuleDef. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vm/src/builtins/module.rs (1)
44-71: Guard against custom create slots returning incomplete modules.
create_modulealways calls__init_dict_from_def, which unwrapsmodule.defandmodule.name. If a custom create slot returns a module without those fields initialized, this will panic. Add an explicit guard (or normalize the module) before continuing.🛡️ Suggested guard
@@ - // Initialize module dict and methods - PyModule::__init_dict_from_def(vm, &module); + // Initialize module dict and methods + if module.def.is_none() || module.name.is_none() { + return Err(vm.new_system_error(format!( + "module '{}' was created without a complete PyModuleDef", + self.name.as_str() + ))); + } + PyModule::__init_dict_from_def(vm, &module); module.__init_methods(vm)?;
PyPayloads are expected to be exposed via PyRef. Copy/Clone for them doesn't make sense for most of cases. If they are used, that must be careful to take risk to confuse it to PyRef clone
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.