diff --git a/crates/stdlib/src/array.rs b/crates/stdlib/src/array.rs index cb6e40d0d..fd3e1bba4 100644 --- a/crates/stdlib/src/array.rs +++ b/crates/stdlib/src/array.rs @@ -33,7 +33,7 @@ pub mod array { }, sequence::{SequenceExt, SequenceMutExt}, sliceable::{ - SaturatedSlice, SequenceIndex, SequenceIndexOp, SliceableSequenceMutOp, + NameArray, SaturatedSlice, SequenceIndex, SequenceIndexOp, SliceableSequenceMutOp, SliceableSequenceOp, }, stdlib::_warnings, @@ -1205,7 +1205,7 @@ pub mod array { } fn getitem_inner(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "array")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self.read().getitem_by_index(i, vm), SequenceIndex::Slice(slice) => self.read().getitem_by_slice(slice, vm), } @@ -1221,7 +1221,7 @@ pub mod array { value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "array")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => { let typecode = zelf.read().typecode(); let item = ArrayContentType::item_from_object(typecode, value, vm)?; @@ -1266,7 +1266,7 @@ pub mod array { } fn delitem_inner(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "array")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self.try_resizable(vm)?.delitem_by_index(i, vm), SequenceIndex::Slice(slice) => self.try_resizable(vm)?.delitem_by_slice(slice, vm), } diff --git a/crates/stdlib/src/elementtree.rs b/crates/stdlib/src/elementtree.rs index 57c793d54..1e49c9ec1 100644 --- a/crates/stdlib/src/elementtree.rs +++ b/crates/stdlib/src/elementtree.rs @@ -29,7 +29,7 @@ pub(crate) mod _elementtree { builtins::{PyDict, PyDictRef, PyList, PyModule, PyStr, PyType, PyTypeRef}, function::{FuncArgs, PySetterValue}, protocol::{PyMappingMethods, PyNumberMethods, PySequenceMethods}, - sliceable::{SequenceIndex, SliceableSequenceOp}, + sliceable::{NameElement, SequenceIndex, SliceableSequenceOp}, types::{ AsMapping, AsNumber, AsSequence, Constructor, DefaultConstructor, Initializer, IterNext, Iterable, Representable, SelfIter, @@ -1028,7 +1028,7 @@ pub(crate) mod _elementtree { } fn sequence_index(vm: &VirtualMachine, needle: &PyObject) -> PyResult { - SequenceIndex::try_from_borrowed_object(vm, needle, "element") + SequenceIndex::try_from_borrowed_object::(vm, needle) .map_err(|_| vm.new_type_error("element indices must be integers")) } diff --git a/crates/stdlib/src/mmap.rs b/crates/stdlib/src/mmap.rs index 2a3ee0bc4..317176c45 100644 --- a/crates/stdlib/src/mmap.rs +++ b/crates/stdlib/src/mmap.rs @@ -18,7 +18,7 @@ mod mmap { protocol::{ BufferDescriptor, BufferMethods, PyBuffer, PyMappingMethods, PySequenceMethods, }, - sliceable::{SaturatedSlice, SequenceIndex, SequenceIndexOp}, + sliceable::{NameMmap, SaturatedSlice, SequenceIndex, SequenceIndexOp}, types::{AsBuffer, AsMapping, AsSequence, Constructor, Representable}, }; use core::ops::{Deref, DerefMut}; @@ -1369,7 +1369,7 @@ mod mmap { } fn getitem_inner(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "mmap")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self.getitem_by_index(i, vm), SequenceIndex::Slice(slice) => self.getitem_by_slice(&slice, vm), } @@ -1381,7 +1381,7 @@ mod mmap { value: &PyObject, vm: &VirtualMachine, ) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "mmap")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => Self::setitem_by_index(zelf, i, value, vm), SequenceIndex::Slice(slice) => Self::setitem_by_slice(zelf, &slice, value, vm), } diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 19e149d9d..9e166ca24 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -28,7 +28,7 @@ use crate::{ BufferDescriptor, BufferFlags, BufferMethods, BufferResizeGuard, PyBuffer, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods, }, - sliceable::{SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp}, + sliceable::{NameBytearray, SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp}, types::{ AsBuffer, AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, DefaultConstructor, Initializer, IterNext, Iterable, PyComparisonOp, Representable, @@ -107,7 +107,7 @@ impl PyByteArray { value: &PyObject, vm: &VirtualMachine, ) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "bytearray")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => zelf._setitem_by_index(i, value, vm), SequenceIndex::Slice(slice) => { let items = if zelf.is(&value) { @@ -126,7 +126,7 @@ impl PyByteArray { } fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "bytearray")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self .borrow_buf() .getitem_by_index(vm, i) @@ -139,7 +139,7 @@ impl PyByteArray { } pub fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "bytearray")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self.try_resizable(vm)?.elements.delitem_by_index(vm, i), SequenceIndex::Slice(slice) => { // TODO: delete 0 elements don't need resizable diff --git a/crates/vm/src/builtins/bytes.rs b/crates/vm/src/builtins/bytes.rs index 415cf4df4..f2d06706b 100644 --- a/crates/vm/src/builtins/bytes.rs +++ b/crates/vm/src/builtins/bytes.rs @@ -22,7 +22,7 @@ use crate::{ BufferDescriptor, BufferFlags, BufferMethods, PyBuffer, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods, }, - sliceable::{SequenceIndex, SliceableSequenceOp}, + sliceable::{NameByte, SequenceIndex, SliceableSequenceOp}, types::{ AsBuffer, AsMapping, AsNumber, AsSequence, Callable, Comparable, Constructor, Hashable, IterNext, Iterable, PyComparisonOp, Representable, SelfIter, @@ -165,7 +165,7 @@ impl PyBytes { } fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "byte")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self .getitem_by_index(vm, i) .map(|x| vm.ctx.new_int(x).into()), diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index d6a2c1669..dbd40b69c 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -17,7 +17,7 @@ use crate::{ protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods}, recursion::ReprGuard, sequence::{MutObjectSequenceOp, OptionalRangeArgs, SequenceExt, SequenceMutExt}, - sliceable::{SaturatedSlice, SequenceIndex, SliceableSequenceOp}, + sliceable::{NameList, SaturatedSlice, SequenceIndex, SliceableSequenceOp}, sorting::timsort, types::{ AsMapping, AsSequence, Comparable, Constructor, Initializer, IterNext, Iterable, @@ -264,7 +264,7 @@ impl PyList { } fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => { let vec = self.borrow_vec(); let pos = vec @@ -363,7 +363,7 @@ impl PyList { } fn _setitem(&self, needle: &PyObject, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(index) => self.assign_item(index, Some(value), vm), SequenceIndex::Slice(slice) => { let items = extract_cloned(&value, self, vm)?; @@ -396,7 +396,7 @@ impl PyList { } fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => self.assign_item(i, None, vm), SequenceIndex::Slice(slice) => self.assign_slice(slice, None, vm), } diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index a1c06e491..c86470573 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -15,7 +15,7 @@ use crate::{ protocol::{PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods}, recursion::ReprGuard, sequence::{OptionalRangeArgs, SequenceExt}, - sliceable::{SequenceIndex, SliceableSequenceOp}, + sliceable::{NameTuple, SequenceIndex, SliceableSequenceOp}, types::{ AsMapping, AsNumber, AsSequence, Comparable, Constructor, Hashable, IterNext, Iterable, PyComparisonOp, Representable, SelfIter, @@ -470,7 +470,7 @@ impl PyTuple { } fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "tuple")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => { let index = self .elements diff --git a/crates/vm/src/sliceable.rs b/crates/vm/src/sliceable.rs index e859c7a3a..d5a897f43 100644 --- a/crates/vm/src/sliceable.rs +++ b/crates/vm/src/sliceable.rs @@ -257,6 +257,34 @@ pub enum SequenceIndex { Slice(SaturatedSlice), } +/// Compile-time type name used in sequence index TypeErrors. +/// +/// `&'static str` is not a legal const-generic type, so the name is an associated +/// constant on a marker type, the same pattern as [`crate::function::ArgName`]. +pub trait SequenceIndexName { + const NAME: &'static str; +} + +macro_rules! sequence_index_name { + ($($ty:ident = $name:literal),* $(,)?) => {$( + #[derive(Clone, Copy, Debug)] + pub struct $ty; + impl SequenceIndexName for $ty { + const NAME: &'static str = $name; + } + )*}; +} + +sequence_index_name! { + NameList = "list", + NameTuple = "tuple", + NameByte = "byte", + NameBytearray = "bytearray", + NameArray = "array", + NameMmap = "mmap", + NameElement = "element", +} + impl SequenceIndex { /// The index or slice `obj` stands for, or `None` when it is neither. fn try_from_object_opt(vm: &VirtualMachine, obj: &PyObject) -> Option> { @@ -280,27 +308,47 @@ impl SequenceIndex { } } - pub fn try_from_borrowed_object( + fn try_from_object( vm: &VirtualMachine, obj: &PyObject, - type_name: &str, + err_fmt: fn(&str) -> String, ) -> PyResult { - Self::try_from_object_opt(vm, obj).unwrap_or_else(|| { - Err(vm.new_type_error(format!( - "{type_name} indices must be integers or slices, not {}", - obj.class().slot_name() - ))) - }) + Self::try_from_object_opt(vm, obj) + .unwrap_or_else(|| Self::index_type_error(vm, obj, err_fmt)) + } + + #[cold] + #[inline(never)] + fn index_type_error( + vm: &VirtualMachine, + obj: &PyObject, + err_fmt: fn(&str) -> String, + ) -> PyResult { + let class_name = obj.class().slot_name(); + Err(vm.new_type_error(err_fmt(&class_name))) + } + + fn sequence_index_type_error(class_name: &str) -> String { + format!( + "{} indices must be integers or slices, not {class_name:.200}", + N::NAME + ) + } + + fn str_index_type_error(class_name: &str) -> String { + format!("string indices must be integers, not '{class_name:.200}'") + } + + pub fn try_from_borrowed_object( + vm: &VirtualMachine, + obj: &PyObject, + ) -> PyResult { + Self::try_from_object(vm, obj, Self::sequence_index_type_error::) } /// `unicode_subscript`, which turns down what it cannot use in its own words. pub fn try_from_str_subscript(vm: &VirtualMachine, obj: &PyObject) -> PyResult { - Self::try_from_object_opt(vm, obj).unwrap_or_else(|| { - Err(vm.new_type_error(format!( - "string indices must be integers, not '{}'", - obj.class().slot_name() - ))) - }) + Self::try_from_object(vm, obj, Self::str_index_type_error) } } diff --git a/crates/vm/src/types/structseq.rs b/crates/vm/src/types/structseq.rs index 098224950..dee57fb92 100644 --- a/crates/vm/src/types/structseq.rs +++ b/crates/vm/src/types/structseq.rs @@ -11,7 +11,7 @@ use crate::{ }, iter::PyExactSizeIterator, protocol::{PyMappingMethods, PySequenceMethods}, - sliceable::{SequenceIndex, SliceableSequenceOp}, + sliceable::{NameTuple, SequenceIndex, SliceableSequenceOp}, types::PyComparisonOp, vm::Context, }; @@ -217,7 +217,7 @@ static STRUCT_SEQUENCE_AS_MAPPING: LazyLock = let tuple = mapping.obj.downcast_ref::().unwrap(); let visible_elements = &tuple.as_slice()[..n_seq]; - match SequenceIndex::try_from_borrowed_object(vm, needle, "tuple")? { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { SequenceIndex::Int(i) => visible_elements.getitem_by_index(vm, i), SequenceIndex::Slice(slice) => visible_elements .getitem_by_slice(vm, slice) @@ -376,7 +376,7 @@ pub trait PyStructSequence: StaticType + PyClassImpl + Sized + 'static { let n_seq = get_visible_len(zelf.as_ref(), vm)?; let visible_elements = &zelf.as_slice()[..n_seq]; - match SequenceIndex::try_from_borrowed_object(vm, &needle, "tuple")? { + match SequenceIndex::try_from_borrowed_object::(vm, &needle)? { SequenceIndex::Int(i) => visible_elements.getitem_by_index(vm, i), SequenceIndex::Slice(slice) => visible_elements .getitem_by_slice(vm, slice) diff --git a/extra_tests/snippets/builtin_list.py b/extra_tests/snippets/builtin_list.py index 189f79477..59f77e5a9 100644 --- a/extra_tests/snippets/builtin_list.py +++ b/extra_tests/snippets/builtin_list.py @@ -1388,3 +1388,22 @@ def test_slice_replacement_lifetime(): test_slice_replacement_lifetime() + + +def test_subscript_typeerror_message(): + def message(index): + try: + [0, 1, 2][index] + except TypeError as err: + return str(err) + raise AssertionError("TypeError was not raised") + + assert message("x") == "list indices must be integers or slices, not str" + + Long = type("A" * 250, (), {}) + assert message(Long()) == "list indices must be integers or slices, not {}".format( + "A" * 200 + ) + + +test_subscript_typeerror_message() diff --git a/extra_tests/snippets/builtin_str.py b/extra_tests/snippets/builtin_str.py index d114e2d57..a8da6ab96 100644 --- a/extra_tests/snippets/builtin_str.py +++ b/extra_tests/snippets/builtin_str.py @@ -1160,3 +1160,25 @@ def test_concat_error_message(): test_concat_error_message() + + +def test_subscript_typeerror_message(): + def message(index): + try: + "abc"[index] + except TypeError as err: + return str(err) + raise AssertionError("TypeError was not raised") + + assert message("def") == "string indices must be integers, not 'str'" + assert message(object()) == "string indices must be integers, not 'object'" + assert message(1.5) == "string indices must be integers, not 'float'" + assert message(None) == "string indices must be integers, not 'NoneType'" + + Long = type("A" * 250, (), {}) + assert message(Long()) == "string indices must be integers, not '{}'".format( + "A" * 200 + ) + + +test_subscript_typeerror_message()