Remove Copy from PyPayload by youknowone · Pull Request #6865 · RustPython/RustPython · GitHub

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
- Added new public methods for module creation and execution on PyModuleDef.
Chores
- Removed workspace linting setting for copy implementations.
- Refactored internal trait implementations across various type definitions to improve memory efficiency and ownership semantics.

✏️ Tip: You can customize this high-level summary in your review settings.

This reverts commit 2f76ef6.

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s)	Summary
Configuration and workspace Cargo.toml	Removed missing_copy_implementations = "warn" workspace lint
Builtin singleton and primitive types crates/vm/src/builtins/singletons.rs, crates/vm/src/builtins/slice.rs, crates/vm/src/builtins/object.rs	Removed Clone, Copy derives from PyNone, PyNotImplemented, PyEllipsis, PyBaseObject
Module definition and related types crates/vm/src/builtins/module.rs	Removed Clone, Copy from PyModuleDef, PyModuleSlots, PyModule; added public methods create_module() and exec_module() to PyModuleDef for phase-based initialization
Namespace and descriptor types crates/vm/src/builtins/namespace.rs, crates/vm/src/builtins/descriptor.rs	Changed PyNamespace from unit-like to empty struct and removed Clone, Copy; removed Clone, Copy from MemberGetter, MemberSetter; removed Clone, Copy from MemberKind
Protocol and method types crates/vm/src/protocol/mapping.rs, crates/vm/src/protocol/sequence.rs, crates/vm/src/protocol/buffer.rs, crates/vm/src/function/method.rs	Removed Clone, Copy from PyMappingMethods, PySequenceMethods, BufferMethods; removed Copy from PyMethodDef and HeapMethodDef
Numeric argument conversion types crates/vm/src/function/number.rs	Removed Clone, Copy from ArgIntoComplex, ArgIntoFloat, ArgIntoBool
String and collection argument types crates/vm/src/anystr.rs, crates/vm/src/builtins/zip.rs, crates/vm/src/dict_inner.rs	Removed Clone, Copy from SplitLinesArgs, ExpandTabsArgs, PyZipNewArgs; removed Clone, Copy from DictSize
Utility and sequence types crates/vm/src/sliceable.rs, crates/vm/src/stdlib/marshal.rs, crates/vm/src/stdlib/os.rs, crates/vm/src/stdlib/typevar.rs, crates/vm/src/stdlib/typing.rs	Removed Clone, Copy from SequenceIndex, SaturatedSliceIter, DumpError, TargetIsDirectory, Generic, NoDefault

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Implement copyslot #6505: Implements copyslot for protocol methods (PyMappingMethods, PySequenceMethods) whose Copy/Clone traits are modified in this PR
Apply some clippy lints #6045: Modifies PyNamespace initialization logic in the same file where PyNamespace struct declaration is updated in this PR
derive Copy where possible #6844: Inverse operation that adds Copy derives to many of the same types being modified here, creating potential conflicts

Suggested reviewers

ShaharNaveh
arihant2math

Poem

🐰 Copy no more, we declare with glee,
Traits refined, now lean and free!
Clone removed from every place,
Ownership reclaims its grace.
Modules craft themselves with care—
A tidy codebase, fresh and fair! ✨

🚥 Pre-merge checks | ✅ 3 ✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Remove Copy from PyPayload' accurately summarizes the main change in the PR, which is reverting Copy/Clone implementations from PyPayload and all dependent public structs and enums throughout the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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_module always calls __init_dict_from_def, which unwraps module.def and module.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)?;

youknowone added 2 commits January 24, 2026 21:41

Revert "Add workspace lint"

0a0427a

This reverts commit 2f76ef6.

Revert Copy/Clone form PyPayload

7322723

youknowone requested a review from ShaharNaveh January 24, 2026 12:45

youknowone marked this pull request as ready for review January 24, 2026 12:47

coderabbitai Bot reviewed Jan 24, 2026

View reviewed changes

youknowone merged commit e0e2926 into RustPython:main Jan 24, 2026
12 of 13 checks passed

youknowone deleted the pypayload-no-copy branch January 24, 2026 13:03

Remove Copy from PyPayload by youknowone · Pull Request #6865 · RustPython/RustPython · GitHub

Uh oh!

Remove Copy from PyPayload - #6865

Remove Copy from PyPayload#6865
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:pypayload-no-copy

youknowone commented Jan 24, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jan 24, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Remove Copy from PyPayload by youknowone · Pull Request #6865 · RustPython/RustPython · GitHub

Uh oh!

Remove Copy from PyPayload - #6865

Remove Copy from PyPayload#6865youknowone merged 2 commits intoRustPython:mainRustPython/RustPython:mainfrom youknowone:pypayload-no-copyyouknowone/RustPython:pypayload-no-copyCopy head branch name to clipboard

Conversation

youknowone commented Jan 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Remove Copy from PyPayload#6865
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:pypayload-no-copy

youknowone commented Jan 24, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jan 24, 2026 •
edited
Loading