GeezerVison is showing github.com — derive `Copy` where possible by ShaharNaveh · Pull Request #6844 · RustPython/RustPython
Conversation
ShaharNaveh
commented
Jan 23, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
-
Chores
- Made many lightweight/internal types trivially copyable to simplify value handling across the codebase.
- Added a workspace lint to surface missing copy implementations as warnings.
- Extended buffer support with explicit retain/release hooks.
-
Bug Fixes
- Improved diagnostics and debug output for thread/lock acquisition scenarios.
✏️ Tip: You can customize this high-level summary in your review settings.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Jan 23, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
📝 Walkthrough
Walkthrough
Adds many Clone/Copy derives across crates, a workspace lint, small API fields (buffer release/retain), a TryLockThreadError and SingleThreadId change, and makes ExceptionTableEntry::new a const fn. No behavioral control-flow changes beyond by-value usage where derives permit it.
Changes
| Cohort / File(s) |
Summary |
Lint Configuration
\Cargo.toml`` |
Added workspace lint: workspace.lints.rust.missing_copy_implementations = "warn". |
Codegen
\crates/codegen/src/compile.rs`, `crates/codegen/src/error.rs`, `crates/codegen/src/ir.rs`, `crates/codegen/src/symboltable.rs`` |
Added Clone, Copy derives to CompileOpts, DoneWithFuture, PatternUnreachableReason, InstructionInfo, ExceptHandlerInfo, SymbolUsage; replaced some .clone() with by-value uses in ir.rs. |
Common — formats/encodings/hash/int/str/cformat
\crates/common/src/format.rs`, `crates/common/src/encodings.rs`, `crates/common/src/hash.rs`, `crates/common/src/int.rs`, `crates/common/src/str.rs`, `crates/common/src/cformat.rs`` |
Added Clone, Copy derives to many small/public types and error structs (Format*, encoding errors, HashSecret, BytesToIntError, UnicodeEscapeCodepoint, CFormatErrorType, CFormatError). |
Common — locking / single-thread id
\crates/common/src/lock/thread_mutex.rs`, `crates/common/src/lock/cell_lock.rs`` |
Added TryLockThreadError (Clone/Copy), introduced SingleThreadId (Clone/Copy) with unsafe impl GetThreadId using NonZero::new_unchecked(1), and adjusted try_lock-related debug paths. |
Compiler-core
\crates/compiler-core/src/bytecode.rs`, `crates/compiler-core/src/marshal.rs`, `crates/compiler-core/src/mode.rs`` |
ExceptionTableEntry now derives Copy and its new is pub const fn; added Clone, Copy to MarshalError and ModeParseError. |
SRE engine
\crates/sre_engine/src/constants.rs`` |
Added Clone, Copy derives to SreOpcode, SreAtCode, SreCatCode, and SreInfo. |
VM — builtins & core types
\crates/vm/src/builtins/.rs`, `crates/vm/src/dict_inner.rs`, `crates/vm/src/sliceable.rs`, `crates/vm/src/function/`, `crates/vm/src/function/number.rs`` |
Widespread addition of Clone, Copy to many VM/public types (singletons, ellipsis, base object, namespace → unit-like, module types, method defs, ArgInto* types, DictSize, SequenceIndex, SaturatedSliceIter, etc.); added MemberGetter and MemberSetter enums. |
VM — protocol & buffers
\crates/vm/src/protocol/buffer.rs`, `crates/vm/src/protocol/mapping.rs`, `crates/vm/src/protocol/sequence.rs`` |
BufferMethods now Clone, Copy and gained release: fn(&PyBuffer) and retain: fn(&PyBuffer) fields; PyMappingMethods and PySequenceMethods gain Clone, Copy. |
VM — stdlib / misc / examples
\crates/vm/src/stdlib/*.rs`, `crates/vm/src/anystr.rs`, `crates/vm/src/builtins/zip.rs`, `crates/wasm/src/vm_class.rs`, `examples/dis.rs`` |
Added Clone, Copy derives to various stdlib/public types (DumpError, TargetIsDirectory, Generic → unit-like, NoDefault, Split/Expand args, PyZipNewArgs, etc.); VMStore gained Clone, Copy; examples/dis.rs stops cloning opts in loop (uses copy). |
Windows fileutils
\crates/common/src/fileutils.rs`` |
Added Clone, Copy to several Windows-specific stat structs and enums to allow by-value copies. |
Sequence Diagram(s)
(Skipped — changes are primarily trait derives, small API additions, and minor signature tweaks that do not introduce a new multi-component sequential flow.)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
🐰
I nudged the structs, light as air,
Gave them Copy feet to skip and share.
No heavy clones, just tiny hops,
A lint gave notice — mind the props.
Now fields can flutter — hop, hop, hop!
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
⚠️ Warning |
Docstring coverage is 31.03% which is insufficient. The required threshold is 80.00%. |
Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The PR title 'derive Copy where possible' accurately and clearly describes the main objective of the changeset—adding Copy trait derivations to multiple types across the codebase. |
✏️ 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.
❤️ Share
Comment @coderabbitai help to get the list of available commands and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/common/src/lock/cell_lock.rs`:
- Around line 210-212: Fix the typo in the safety comment inside the
nonzero_thread_id function: change "Saftey: This is constant." to "Safety: This
is constant." near the NonZero::new_unchecked(1) call so the safety rationale is
spelled correctly for function nonzero_thread_id and the unsafe block using
NonZero::new_unchecked is properly documented.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|