diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 7ab4c810a08..c6e86fe401e 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -2527,7 +2527,6 @@ def test_incorrect_constructor(self): args = ("bad.py", 1, 2, "abcdefg", 1) self.assertRaises(TypeError, SyntaxError, "bad bad", args) - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 2 is not None def test_syntax_error_memory_leak(self): # gh-146250: memory leak with re-initialization of SyntaxError e = SyntaxError("msg", ("file.py", 1, 2, "txt", 2, 3)) diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index cd1eb1adbcb..eb3dd880821 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -9,7 +9,7 @@ use crate::{ }, class::{PyClassImpl, StaticType}, convert::{IntoPyException, ToPyException, ToPyObject}, - function::{ArgIterable, FuncArgs, IntoFuncArgs, PySetterValue}, + function::{ArgIterable, FuncArgs, IntoFuncArgs}, py_io::{self, Write}, stdlib::sys, suggestion::offer_suggestions, @@ -1029,21 +1029,7 @@ impl ExceptionZoo { excs.python_finalization_error ); - extend_exception!(PySyntaxError, ctx, excs.syntax_error, { - "msg" => ctx.new_static_getset( - "msg", - excs.syntax_error, - make_arg_getter(0), - syntax_error_set_msg, - ), - // TODO: members - "filename" => ctx.none(), - "lineno" => ctx.none(), - "end_lineno" => ctx.none(), - "offset" => ctx.none(), - "end_offset" => ctx.none(), - "text" => ctx.none(), - }); + extend_exception!(PySyntaxError, ctx, excs.syntax_error); extend_exception!(PyIncompleteInputError, ctx, excs.incomplete_input_error); extend_exception!(PyIndentationError, ctx, excs.indentation_error); extend_exception!(PyTabError, ctx, excs.tab_error); @@ -1082,20 +1068,6 @@ fn make_arg_getter(idx: usize) -> impl Fn(PyBaseExceptionRef) -> Option new_args[0] = value, - PySetterValue::Delete => new_args[0] = vm.ctx.none(), - } - *args = PyTuple::new_ref(new_args, &vm.ctx); -} - #[cfg(feature = "serde")] pub struct SerializeException<'vm, 's> { vm: &'vm VirtualMachine, @@ -2551,12 +2523,64 @@ pub(super) mod types { #[repr(transparent)] pub struct PyPythonFinalizationError(PyRuntimeError); - #[pyexception(name, base = PyException, ctx = "syntax_error")] - #[derive(Debug)] - #[repr(transparent)] - pub struct PySyntaxError(PyException); + #[pyexception(name, base = PyException, ctx = "syntax_error", traverse = "manual")] + #[repr(C)] + pub struct PySyntaxError { + base: PyException, + msg: PyAtomicRef>, + filename: PyAtomicRef>, + lineno: PyAtomicRef>, + offset: PyAtomicRef>, + text: PyAtomicRef>, + end_lineno: PyAtomicRef>, + end_offset: PyAtomicRef>, + print_file_and_line: PyAtomicRef>, + } - #[pyexception(with(Initializer))] + impl crate::class::PySubclass for PySyntaxError { + type Base = PyException; + fn as_base(&self) -> &Self::Base { + &self.base + } + } + + impl core::fmt::Debug for PySyntaxError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("PySyntaxError").finish_non_exhaustive() + } + } + + unsafe impl Traverse for PySyntaxError { + fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { + self.base.0.traverse(tracer_fn); + if let Some(obj) = self.msg.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.filename.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.lineno.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.offset.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.text.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.end_lineno.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.end_offset.deref() { + tracer_fn(obj); + } + if let Some(obj) = self.print_file_and_line.deref() { + tracer_fn(obj); + } + } + } + + #[pyexception(with(Constructor, Initializer))] impl PySyntaxError { #[pymethod] fn __str__(zelf: &Py, vm: &VirtualMachine) -> PyStrRef { @@ -2620,58 +2644,196 @@ pub(super) mod types { vm.ctx.new_str(msg_with_location_info) } + + #[pygetset] + fn msg(&self) -> Option { + self.msg.to_owned() + } + + #[pygetset(setter)] + fn set_msg(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.msg.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn filename(&self) -> Option { + self.filename.to_owned() + } + + #[pygetset(setter)] + fn set_filename(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.filename.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn lineno(&self) -> Option { + self.lineno.to_owned() + } + + #[pygetset(setter)] + fn set_lineno(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.lineno.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn offset(&self) -> Option { + self.offset.to_owned() + } + + #[pygetset(setter)] + fn set_offset(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.offset.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn text(&self) -> Option { + self.text.to_owned() + } + + #[pygetset(setter)] + fn set_text(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.text.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn end_lineno(&self) -> Option { + self.end_lineno.to_owned() + } + + #[pygetset(setter)] + fn set_end_lineno(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.end_lineno.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn end_offset(&self) -> Option { + self.end_offset.to_owned() + } + + #[pygetset(setter)] + fn set_end_offset(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.end_offset.swap_to_temporary_refs(value, vm); + } + + #[pygetset] + fn print_file_and_line(&self) -> Option { + self.print_file_and_line.to_owned() + } + + #[pygetset(setter)] + fn set_print_file_and_line(&self, value: PySetterValue, vm: &VirtualMachine) { + let value = match value { + PySetterValue::Assign(v) => Some(v), + PySetterValue::Delete => None, + }; + self.print_file_and_line.swap_to_temporary_refs(value, vm); + } + } + + impl Constructor for PySyntaxError { + type Args = FuncArgs; + + fn py_new(_cls: &Py, args: FuncArgs, vm: &VirtualMachine) -> PyResult { + // msg must also be set here, not only in slot_init: second-level + // subclasses such as TabError never reach PySyntaxError::slot_init. + let msg = args.args.first().cloned(); + let base_exception = PyBaseException::new(args.args, vm); + Ok(Self { + base: PyException(base_exception), + msg: msg.into(), + filename: None.into(), + lineno: None.into(), + offset: None.into(), + text: None.into(), + end_lineno: None.into(), + end_offset: None.into(), + print_file_and_line: None.into(), + }) + } } impl Initializer for PySyntaxError { type Args = FuncArgs; fn slot_init(zelf: PyObjectRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - let len = args.args.len(); - let new_args = args; + let msg = args.args.first().cloned(); + let location_arg = (args.args.len() == 2).then(|| args.args[1].clone()); - zelf.set_attr("print_file_and_line", vm.ctx.none(), vm)?; + PyBaseException::slot_init(zelf.clone(), args, vm)?; + let exc: &Py = zelf.downcast_ref::().unwrap(); - if len == 2 - && let Ok(location_tuple) = new_args.args[1] - .clone() - .downcast::() - { - let location_tup_len = location_tuple.len(); + if let Some(msg) = msg { + exc.msg.swap_to_temporary_refs(Some(msg), vm); + } - match location_tup_len { + // SyntaxError(msg, (filename, lineno, offset, text[, end_lineno, end_offset])) + // The location argument is coerced from any sequence like CPython's + // PySequence_Tuple; a non-sequence raises TypeError. + if let Some(location_arg) = location_arg { + let location: Vec = location_arg.try_to_value(vm)?; + + match location.len() { 4 | 6 => {} 5 => { return Err(vm.new_type_error( "end_offset must be provided when end_lineno is provided", )); } - _ => { + len => { return Err(vm.new_type_error(format!( - "function takes exactly 4 or 6 arguments ({location_tup_len} given)" + "function takes exactly 4 or 6 arguments ({len} given)" ))); } } - for (i, &attr) in [ - "filename", - "lineno", - "offset", - "text", - "end_lineno", - "end_offset", - ] - .iter() - .enumerate() - { - if location_tup_len > i { - zelf.set_attr(attr, location_tuple[i].to_owned(), vm)?; - } else { - break; - } + exc.end_lineno.swap_to_temporary_refs(None, vm); + exc.end_offset.swap_to_temporary_refs(None, vm); + + exc.filename + .swap_to_temporary_refs(Some(location[0].clone()), vm); + exc.lineno + .swap_to_temporary_refs(Some(location[1].clone()), vm); + exc.offset + .swap_to_temporary_refs(Some(location[2].clone()), vm); + exc.text + .swap_to_temporary_refs(Some(location[3].clone()), vm); + if location.len() == 6 { + exc.end_lineno + .swap_to_temporary_refs(Some(location[4].clone()), vm); + exc.end_offset + .swap_to_temporary_refs(Some(location[5].clone()), vm); } } - PyBaseException::slot_init(zelf, new_args, vm) + Ok(()) } fn init(_zelf: PyRef, _args: Self::Args, _vm: &VirtualMachine) -> PyResult<()> { diff --git a/crates/vm/src/vm/vm_new.rs b/crates/vm/src/vm/vm_new.rs index 48909c1a41e..a8acb86f4a5 100644 --- a/crates/vm/src/vm/vm_new.rs +++ b/crates/vm/src/vm/vm_new.rs @@ -341,12 +341,11 @@ impl VirtualMachine { /// [`vm.invoke_exception()`][Self::invoke_exception] or /// [`exceptions::ExceptionCtor`][crate::exceptions::ExceptionCtor] instead. pub fn new_exception(&self, exc_type: PyTypeRef, args: Vec) -> PyBaseExceptionRef { - debug_assert_eq!( - exc_type.slots.basicsize, - core::mem::size_of::(), - "vm.new_exception() is only for exception types without additional payload. The given type '{}' is not allowed. Use vm.new_os_subtype_error() for OSError subtypes.", - exc_type.name() - ); + if exc_type.slots.basicsize != core::mem::size_of::() { + // If constructing the exception raises (e.g. __init__ rejects the + // args), surface that exception instead of panicking. + return self.invoke_exception(&exc_type, args).unwrap_or_else(|e| e); + } PyBaseException::new(args, self) .into_ref_with_type_lazy_dict(self, exc_type)