Skip to content

Remove redundant to_owned() calls#8077

Merged
youknowone merged 1 commit into
RustPython:mainfrom
ShaharNaveh:remove-redundunt-to-owned
Jun 12, 2026
Merged

Remove redundant to_owned() calls#8077
youknowone merged 1 commit into
RustPython:mainfrom
ShaharNaveh:remove-redundunt-to-owned

Conversation

@ShaharNaveh

@ShaharNaveh ShaharNaveh commented Jun 11, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Summary by CodeRabbit

  • Refactor
    • Simplified internal error-message construction across many modules to reduce unnecessary allocations and improve efficiency; no changes to error text, behavior, or public APIs.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 81859942-c06c-4e72-9d9b-6f0b47b981fc

📥 Commits

Reviewing files that changed from the base of the PR and between cc33fe9 and 2da4513.

📒 Files selected for processing (18)
  • crates/capi/src/abstract_.rs
  • crates/capi/src/unicodeobject.rs
  • crates/stdlib/src/multiprocessing.rs
  • crates/stdlib/src/select.rs
  • crates/stdlib/src/socket.rs
  • crates/stdlib/src/ssl.rs
  • crates/vm/src/builtins/code.rs
  • crates/vm/src/builtins/function.rs
  • crates/vm/src/builtins/list.rs
  • crates/vm/src/exceptions.rs
  • crates/vm/src/function/time.rs
  • crates/vm/src/stdlib/_signal.rs
  • crates/vm/src/stdlib/marshal.rs
  • crates/vm/src/stdlib/nt.rs
  • crates/vm/src/stdlib/os.rs
  • crates/vm/src/stdlib/sys.rs
  • crates/vm/src/stdlib/time.rs
  • crates/vm/src/vm/mod.rs
✅ Files skipped from review due to trivial changes (11)
  • crates/capi/src/abstract_.rs
  • crates/vm/src/function/time.rs
  • crates/vm/src/stdlib/nt.rs
  • crates/stdlib/src/ssl.rs
  • crates/vm/src/stdlib/sys.rs
  • crates/vm/src/stdlib/os.rs
  • crates/stdlib/src/multiprocessing.rs
  • crates/vm/src/builtins/list.rs
  • crates/vm/src/stdlib/marshal.rs
  • crates/stdlib/src/socket.rs
  • crates/vm/src/stdlib/_signal.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • crates/vm/src/vm/mod.rs
  • crates/vm/src/exceptions.rs
  • crates/stdlib/src/select.rs
  • crates/vm/src/builtins/code.rs
  • crates/vm/src/stdlib/time.rs
📝 Walkthrough

Walkthrough

This PR replaces many small constant error-message allocations with direct string literals passed to VM error constructors across multiple modules, without changing error texts or control flow.

Changes

Error message string literal optimizations

Layer / File(s) Summary
CAPI error construction simplification
crates/capi/src/abstract_.rs, crates/capi/src/unicodeobject.rs		 C API boundary functions now pass error messages as string literals to vm.new_system_error and vm.new_type_error, removing .to_owned() allocations.
Stdlib error simplifications
crates/stdlib/src/multiprocessing.rs, crates/stdlib/src/select.rs, crates/stdlib/src/socket.rs, crates/stdlib/src/ssl.rs		 Multiprocessing timeout, epoll flags validation, many socket/address/service/protocol lookup and validation paths, and SSL recv-result validation now construct OS errors from string literals without temporary allocations.
VM builtins error optimizations
crates/vm/src/builtins/code.rs, crates/vm/src/builtins/function.rs, crates/vm/src/builtins/list.rs, crates/vm/src/exceptions.rs, crates/vm/src/function/time.rs		 Code object loading, bound-method validation, list assignment index errors, syntax-error slot validation, and timeout NaN validation now use string literals directly in error constructors.
VM stdlib module error simplifications
crates/vm/src/stdlib/_signal.rs, crates/vm/src/stdlib/marshal.rs, crates/vm/src/stdlib/nt.rs, crates/vm/src/stdlib/os.rs, crates/vm/src/stdlib/sys.rs, crates/vm/src/stdlib/time.rs		 Signal setup, marshal validation (multiple cases), NT getlogin, OS timing/loadavg, sandbox I/O errors, and platform-specific time functions now eliminate temporary string allocations in error construction.
VM core initialization error optimization
crates/vm/src/vm/mod.rs		 VM initialization's callable cache error now passes the message as a string literal to vm.new_runtime_error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • youknowone
  • bschoenmaeckers

Poem

🐇 I nibble at strings with swift delight,
I swap owned crumbs for literals light,
No heap to dig, no clutter in the burrow,
Errors hop tidy, the code runs thorough,
A happy rabbit, keeping builds bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.27% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Remove redundant to_owned() calls' clearly summarizes the main change across all modified files, which consistently replace .to_owned() allocations with direct string literal references.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@ShaharNaveh ShaharNaveh changed the title Remove redundunt to_owned() calls Remove redundant to_owned() calls Jun 11, 2026
@ShaharNaveh ShaharNaveh force-pushed the remove-redundunt-to-owned branch from cc33fe9 to 2da4513 Compare June 11, 2026 11:55
@youknowone youknowone merged commit b35c5c3 into RustPython:main Jun 12, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@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