native ExceptionGroup by youknowone · Pull Request #6358 · RustPython/RustPython

1-12: LGTM!

Clean module documentation and well-organized imports. The separation of concerns is appropriate.

14-33: LGTM!

The dynamic type creation correctly establishes multiple inheritance from BaseExceptionGroup and Exception, which is required for Python 3.11+ ExceptionGroup semantics. The use of expect() is acceptable here as this is initialization-time code where failure should be fatal.

62-70: LGTM!

Proper validation requiring exactly 2 positional arguments, matching CPython's BaseExceptionGroup.__new__() behavior.

81-98: Good validation, but consider consolidating error messages.

The validation correctly rejects sets, frozensets, and None before attempting sequence conversion. The error handling is robust.

107-122: LGTM!

Correct validation ensuring all items are BaseException instances and tracking whether any non-Exception items exist for auto-conversion logic.

124-160: LGTM!

The class selection logic correctly handles:

1. ExceptionGroup - rejects BaseExceptions that aren't Exceptions
2. BaseExceptionGroup - auto-converts to ExceptionGroup when all are Exceptions
3. User-defined subclasses - enforces nesting constraints based on inheritance

This matches CPython 3.11+ behavior.

162-171: The no-op __init__ is intentional and correct.

The comment explains the rationale well. Since __new__ already sets up the args tuple correctly, __init__ should be a no-op to allow subclasses flexibility.

187-232: LGTM!

The subgroup implementation correctly:

• Returns self if the entire group matches
• Recursively processes nested groups
• Tracks modifications to avoid unnecessary allocations
• Returns None for empty matches
• Copies metadata via derive_and_copy_attributes

234-291: LGTM!

The split implementation correctly partitions exceptions into matching and non-matching groups, handling nested groups recursively. The tuple return format (match, rest) matches CPython.

293-312: LGTM!

Clean __str__ implementation with proper pluralization of "sub-exception(s)".

354-369: LGTM!

Helper functions are clean and appropriately scoped within the types module.

371-428: LGTM!

The ConditionMatcher enum and get_condition_matcher function properly handle all three condition types:

1. Single exception type
2. Tuple of exception types
3. Callable predicate

The check method correctly implements the matching logic for each variant.

430-468: LGTM!

The derive_and_copy_attributes function properly:

• Creates a new group via derive
• Validates the return type
• Copies __traceback__, __context__, __cause__
• Copies __notes__ as a new list (avoiding shared mutation)

This preserves exception metadata through subgroup/split operations as required by PEP 654.

173-185: Use the instance's actual class for derive instead of always BaseExceptionGroup.

The derive method always creates a BaseExceptionGroup, but CPython's behavior (per PEP 654) is to use the derived class's constructor. This ensures that subclasses of BaseExceptionGroup preserve their type when subgroup() and split() perform partitioning operations.

Consider:

 fn derive(
     zelf: PyRef<PyBaseException>,
     excs: PyObjectRef,
     vm: &VirtualMachine,
 ) -> PyResult {
     let message = zelf.get_arg(0).unwrap_or_else(|| vm.ctx.new_str("").into());
+    let cls = zelf.class().to_owned();
     vm.invoke_exception(
-        vm.ctx.exceptions.base_exception_group.to_owned(),
+        cls,
         vec![message, excs],
     )
     .map(|e| e.into())
 }

35-40: Verify that Context::genesis() is safe to call during static initialization and that this pattern aligns with RustPython's VM initialization best practices.

The static_cell! macro itself is a safe pattern for runtime-initialized statics. However, ensure that Context::genesis() has no dependencies on VM state that might not be available during early static initialization, and confirm this approach is the intended pattern for exception type registration rather than initializing types during Interpreter::with_init.