Add new_payload_exception helper for constructing built-in payload exceptions - #8403
Add new_payload_exception helper for constructing built-in payload exceptions#8403kangdora wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
@kangdora do you have any concern to work more on this patch? |
|
Yeah, one concern. Since Honestly this is a little beyond my current level, so instead of changing everything at once I've been going through the call sites to figure out which ones are actually safe to touch first. Where I've landed so far:
So rather than limiting it upfront, I'd like to keep working through these gradually as I understand the area better. If I've misjudged any of these boundaries, I'd really appreciate a correction. |
Follow-up to the discussion in #8348.
Built-in exceptions that carry a payload (a
#[repr(C)]struct with extrafields) are currently constructed either through
PyType::call(via
invoke_exception) or through a bespoke per-type builder likeOSErrorBuilder. This adds a generic helper for the internal builders thatknow their exact type at compile time, so they can construct directly
(
py_new+slot_init) without going throughPyType::call:new_exceptionstays as the thin, restricted fast path for payload-freetypes, and
invoke_exceptionstill handles the runtime-typed / user-subclasspath (
raise <expr>, C-API) — a helper parameterized on a compile-timeTcan't respect a user subclass's overridden
__new__/__init__.Error handling: only
into_ref_with_type_lazy_dictis.expected — itfails when
clsdoesn't structurally matchT, which is a caller mistake thatuser input can't cause.
py_newandslot_initpropagate with?, so alegitimately-rejected argument (e.g.
__init__refusing a bad value) surfacesas a normal Python exception.
Routes
new_stop_iterationandOSErrorBuilderthrough the helper as thefirst two call sites — the simplest and most complex payload exceptions
respectively. Behaves identically to before and simplifies
OSErrorBuilder.Open questions
Opening as a draft mainly to get feedback on a couple of points:
new_stop_iterationreturnPyBaseExceptionRef, so the helper'sPyRef<T>needs an owned upcast. I used.upcast(), which goes through a runtime downcast. The zero-cost conversions(
upcast_ref/to_base) only yield&Py<_>, andinto_basedowncasts too, soI don't see a zero-cost owned conversion here — is the downcast fine to keep?
Constructor<Args = FuncArgs> + Initializerdoesn't actuallyconstrain
Tto an exception type. Not sure whether that's worth tightening.