Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2453,6 +2453,14 @@ def test_attributes_old_constructor(self):
self.assertEqual(error, the_exception.text)
self.assertEqual("bad bad", the_exception.msg)

def test_msg_attribute_writable(self):
err = SyntaxError("bad bad", ("bad.py", 1, 2, "abcdefg"))
err.msg = "changed"
self.assertEqual(err.msg, "changed")
self.assertEqual(str(err), "changed (bad.py, line 1)")
del err.msg
self.assertIsNone(err.msg)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must not add our own test under Lib/test/.

Please move it to extra_tests/snippets/builtin_exceptions.py

Also, please add a sentence to copilot instruction not to make this happen again.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 9ba97e5: moved the SyntaxError msg regression check to extra_tests/snippets/builtin_exceptions.py and added guidance in DEVELOPMENT.md to keep RustPython-specific tests out of Lib/test.

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_incorrect_constructor(self):
args = ("bad.py", 1, 2)
Expand Down
45 changes: 34 additions & 11 deletions crates/vm/src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
},
class::{PyClassImpl, StaticType},
convert::{ToPyException, ToPyObject},
function::{ArgIterable, FuncArgs, IntoFuncArgs},
function::{ArgIterable, FuncArgs, IntoFuncArgs, PySetterValue},
py_io::{self, Write},
stdlib::sys,
suggestion::offer_suggestions,
Expand Down Expand Up @@ -994,7 +994,12 @@ impl ExceptionZoo {
extend_exception!(PyRecursionError, ctx, excs.recursion_error);

extend_exception!(PySyntaxError, ctx, excs.syntax_error, {
"msg" => ctx.new_readonly_getset("msg", excs.syntax_error, make_arg_getter(0)),
"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(),
Expand Down Expand Up @@ -1041,6 +1046,25 @@ fn make_arg_getter(idx: usize) -> impl Fn(PyBaseExceptionRef) -> Option<PyObject
move |exc| exc.get_arg(idx)
}

fn syntax_error_set_msg(
exc: PyBaseExceptionRef,
value: PySetterValue,
vm: &VirtualMachine,
) -> PyResult<()> {
let mut args = exc.args.write();
let mut new_args = args.as_slice().to_vec();
// Ensure the message slot at index 0 always exists for SyntaxError.args.
if new_args.is_empty() {
new_args.push(vm.ctx.none());
}
match value {
PySetterValue::Assign(value) => new_args[0] = value,
PySetterValue::Delete => new_args[0] = vm.ctx.none(),
}
*args = PyTuple::new_ref(new_args, &vm.ctx);
Ok(())
}

fn system_exit_code(exc: PyBaseExceptionRef) -> Option<PyObjectRef> {
exc.args.read().first().map(|code| {
match_class!(match code {
Expand Down Expand Up @@ -2048,15 +2072,14 @@ pub(super) mod types {
.unwrap_or_else(|_| vm.ctx.new_str("<filename str() failed>"))
});

let args = zelf.args();

let msg = if args.len() == 1 {
vm.exception_args_as_string(args, false)
.into_iter()
.exactly_one()
.unwrap()
} else {
return zelf.__str__(vm);
let msg = match zelf.as_object().get_attr("msg", vm) {
Ok(obj) => obj
.str(vm)
.unwrap_or_else(|_| vm.ctx.new_str("<msg str() failed>")),
Err(_) => {
// Fallback to the base formatting if the msg attribute was deleted or attribute lookup fails for any reason.
return Py::<PyBaseException>::__str__(zelf, vm);
}
};

let msg_with_location_info: String = match (maybe_lineno, maybe_filename) {
Expand Down
Loading