Take a borrow in invoke_exception instead of an owned type#8327
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthrough
ChangesException type borrowing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
&Py<PyType>instead ofPyTypeRefinvm.invoke_exception()#8320Summary
VirtualMachine::invoke_exceptiontook its class argument as an ownedPyTypeRef, but it never takes ownership — it only borrowsclsforPyType::call(which itself is defined asfn call(zelf: &Py<Self>, ...)) and forDisplayin the error message.clsis never moved or stored.Because the signature demanded ownership, callers holding only a borrow had to
.to_owned()— an atomic refcount bump and later decrement — purely to satisfy the type, even though the reference was dropped immediately after the call. This changes the parameter to&Py<PyType>, the exact borrowed form (PyTypeRef=PyRef<PyType>derefs toPy<PyType>), and drops those.to_owned()calls.Call sites updated:
stdlib/_io.rs(3×)vm.ctx.exceptions.blocking_io_error.to_owned()stdlib/sys.rsvm.ctx.exceptions.system_exit.to_owned()stdlib/builtins.rsvm.ctx.exceptions.system_exit.to_owned()stdlib/_thread.rsvm.ctx.exceptions.system_exit.to_owned()vm/mod.rsvm.ctx.exceptions.system_exit.to_owned()vm/vm_new.rsvm.ctx.exceptions.stop_iteration.to_owned()exception_group.rsvm.ctx.exceptions.base_exception_group.to_owned()exceptions.rstyp.to_owned()capi/pyerrors.rsexc_type.to_owned()ExceptionCtor::instantiate/instantiate_valueown theirclsvia theselfmatch, so they just pass&cls— no clone either way.No behavior change; this is an API/ergonomics cleanup that removes unnecessary refcount churn and makes the helper consistent with the
PyType::callit wraps.Test plan
test_exceptions,test_sys,test_io(667 tests, covering theBlockingIOErrorsites),extra_tests/snippets/builtin_exceptions.py: no regressions (the singletest_exceptionsfailure,test_badisinstance, is pre-existing onmain).sys.exit/exit()/_thread.exit,raise ValueErrorandraise ValueError(...)viaExceptionCtor,ExceptionGroup, generatorStopIteration,OSErrorfrom a failedopen, and the genuineErrpath ofinvoke_exception(aBaseExceptionsubclass whose__new__returns a non-exception).cargo fmt --check/cargo clippy: clean.Assisted-by: Claude Code:claude-fable-5
Summary by CodeRabbit
SystemExit,BlockingIOError,StopIteration, COM errors, and other exception paths.