Skip to content

derive Copy where possible #6844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
youknowone merged 3 commits into from
Jan 23, 2026
Merged

derive Copy where possible #6844

youknowone merged 3 commits into from
Jan 23, 2026

Conversation

@ShaharNaveh
Copy link
Contributor

@ShaharNaveh ShaharNaveh commented Jan 23, 2026 •
edited by coderabbitai bot
Loading

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026 •
edited
Loading

📝 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

  • youknowone
  • arihant2math

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
  • 📝 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.

coderabbitai[bot]
coderabbitai bot reviewed Jan 23, 2026
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

@ShaharNaveh ShaharNaveh force-pushed the codegen-blocks-copy branch 2 times, most recently from ce752a9 to 000fff0 Compare January 23, 2026 13:50
@youknowone youknowone added this pull request to the merge queue Jan 23, 2026
Merged via the queue into RustPython:main with commit af7bbfa Jan 23, 2026
13 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ShaharNaveh @youknowone