Skip to content

Commit ab09de8

Browse files
ShaharNavehShaharNaveh
authored
Remove unnecessary string conversions in error message construction (#5826)
* Make `new_X_error` recv `impl Into<String>` * Maks the rest of the methods to be generic * Remove `.to_owned()` from more files
1 parent 6a992d4 commit ab09de8

File tree

18 files changed

+132
-158
lines changed

18 files changed

+132
-158
lines changed

‎stdlib/src/array.rs‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ mod array {
581581
.chars()
582582
.exactly_one()
583583
.map(|ch| Self(ch as _))
584-
.map_err(|_| vm.new_type_error("array item must be unicode character".into()))
584+
.map_err(|_| vm.new_type_error("array item must be unicode character"))
585585
}
586586
fn byteswap(self) -> Self {
587587
Self(self.0.swap_bytes())
@@ -832,9 +832,9 @@ mod array {
832832
))
833833
})?;
834834
if zelf.read().typecode() != 'u' {
835-
return Err(vm.new_value_error(
836-
"fromunicode() may only be called on unicode type arrays".into(),
837-
));
835+
return Err(
836+
vm.new_value_error("fromunicode() may only be called on unicode type arrays")
837+
);
838838
}
839839
let mut w = zelf.try_resizable(vm)?;
840840
let bytes = Self::_unicode_to_wchar_bytes(wtf8, w.itemsize());
@@ -846,9 +846,9 @@ mod array {
846846
fn tounicode(&self, vm: &VirtualMachine) -> PyResult<Wtf8Buf> {
847847
let array = self.array.read();
848848
if array.typecode() != 'u' {
849-
return Err(vm.new_value_error(
850-
"tounicode() may only be called on unicode type arrays".into(),
851-
));
849+
return Err(
850+
vm.new_value_error("tounicode() may only be called on unicode type arrays")
851+
);
852852
}
853853
let bytes = array.get_bytes();
854854
Self::_wchar_bytes_to_string(bytes, self.itemsize(), vm)
@@ -1500,7 +1500,7 @@ mod array {
15001500
.unwrap_or(u8::MAX)
15011501
.try_into()
15021502
.map_err(|_| {
1503-
vm.new_value_error("third argument must be a valid machine format code.".into())
1503+
vm.new_value_error("third argument must be a valid machine format code.")
15041504
})
15051505
}
15061506
}
@@ -1572,11 +1572,11 @@ mod array {
15721572
fn check_type_code(spec: PyStrRef, vm: &VirtualMachine) -> PyResult<ArrayContentType> {
15731573
let spec = spec.as_str().chars().exactly_one().map_err(|_| {
15741574
vm.new_type_error(
1575-
"_array_reconstructor() argument 2 must be a unicode character, not str".into(),
1575+
"_array_reconstructor() argument 2 must be a unicode character, not str",
15761576
)
15771577
})?;
15781578
ArrayContentType::from_char(spec)
1579-
.map_err(|_| vm.new_value_error("second argument must be a valid type code".into()))
1579+
.map_err(|_| vm.new_value_error("second argument must be a valid type code"))
15801580
}
15811581

15821582
macro_rules! chunk_to_obj {
@@ -1609,7 +1609,7 @@ mod array {
16091609
let format = args.mformat_code;
16101610
let bytes = args.items.as_bytes();
16111611
if bytes.len() % format.item_size() != 0 {
1612-
return Err(vm.new_value_error("bytes length not a multiple of item size".into()));
1612+
return Err(vm.new_value_error("bytes length not a multiple of item size"));
16131613
}
16141614
if MachineFormatCode::from_typecode(array.typecode()) == Some(format) {
16151615
array.frombytes(bytes);
@@ -1642,9 +1642,8 @@ mod array {
16421642
})?,
16431643
MachineFormatCode::Utf16 { big_endian } => {
16441644
let utf16: Vec<_> = chunks.map(|b| chunk_to_obj!(b, u16, big_endian)).collect();
1645-
let s = String::from_utf16(&utf16).map_err(|_| {
1646-
vm.new_unicode_encode_error("items cannot decode as utf16".into())
1647-
})?;
1645+
let s = String::from_utf16(&utf16)
1646+
.map_err(|_| vm.new_unicode_encode_error("items cannot decode as utf16"))?;
16481647
let bytes = PyArray::_unicode_to_wchar_bytes((*s).as_ref(), array.itemsize());
16491648
array.frombytes_move(bytes);
16501649
}

‎stdlib/src/hashlib.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ pub mod _hashlib {
108108

109109
#[pyslot]
110110
fn slot_new(_cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
111-
Err(vm.new_type_error("cannot create '_hashlib.HASH' instances".into()))
111+
Err(vm.new_type_error("cannot create '_hashlib.HASH' instances"))
112112
}
113113

114114
#[pygetset]
@@ -181,7 +181,7 @@ pub mod _hashlib {
181181

182182
#[pyslot]
183183
fn slot_new(_cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
184-
Err(vm.new_type_error("cannot create '_hashlib.HASHXOF' instances".into()))
184+
Err(vm.new_type_error("cannot create '_hashlib.HASHXOF' instances"))
185185
}
186186

187187
#[pygetset]
@@ -347,7 +347,7 @@ pub mod _hashlib {
347347

348348
#[pyfunction]
349349
fn hmac_new(_args: NewHMACHashArgs, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
350-
Err(vm.new_type_error("cannot create 'hmac' instances".into())) // TODO: RUSTPYTHON support hmac
350+
Err(vm.new_type_error("cannot create 'hmac' instances")) // TODO: RUSTPYTHON support hmac
351351
}
352352

353353
pub trait ThreadSafeDynDigest: DynClone + DynDigest + Sync + Send {}

‎vm/src/buffer.rs‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -497,15 +497,12 @@ fn get_int_or_index<T>(vm: &VirtualMachine, arg: PyObjectRef) -> PyResult<T>
497497
where
498498
T: PrimInt + for<'a> TryFrom<&'a BigInt>,
499499
{
500-
let index = arg.try_index_opt(vm).unwrap_or_else(|| {
501-
Err(new_struct_error(
502-
vm,
503-
"required argument is not an integer".to_owned(),
504-
))
505-
})?;
500+
let index = arg
501+
.try_index_opt(vm)
502+
.unwrap_or_else(|| Err(new_struct_error(vm, "required argument is not an integer")))?;
506503
index
507504
.try_to_primitive(vm)
508-
.map_err(|_| new_struct_error(vm, "argument out of range".to_owned()))
505+
.map_err(|_| new_struct_error(vm, "argument out of range"))
509506
}
510507

511508
make_pack_prim_int!(i8);
@@ -549,7 +546,7 @@ impl Packable for f16 {
549546
// "from_f64 should be preferred in any non-`const` context" except it gives the wrong result :/
550547
let f_16 = f16::from_f64_const(f_64);
551548
if f_16.is_infinite() != f_64.is_infinite() {
552-
return Err(vm.new_overflow_error("float too large to pack with e format".to_owned()));
549+
return Err(vm.new_overflow_error("float too large to pack with e format"));
553550
}
554551
f_16.to_bits().pack_int::<E>(data);
555552
Ok(())
@@ -586,12 +583,11 @@ impl Packable for bool {
586583

587584
fn pack_char(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()> {
588585
let v = PyBytesRef::try_from_object(vm, arg)?;
589-
let ch = *v.as_bytes().iter().exactly_one().map_err(|_| {
590-
new_struct_error(
591-
vm,
592-
"char format requires a bytes object of length 1".to_owned(),
593-
)
594-
})?;
586+
let ch = *v
587+
.as_bytes()
588+
.iter()
589+
.exactly_one()
590+
.map_err(|_| new_struct_error(vm, "char format requires a bytes object of length 1"))?;
595591
data[0] = ch;
596592
Ok(())
597593
}
@@ -647,8 +643,8 @@ pub fn struct_error_type(vm: &VirtualMachine) -> &'static PyTypeRef {
647643
INSTANCE.get_or_init(|| vm.ctx.new_exception_type("struct", "error", None))
648644
}
649645

650-
pub fn new_struct_error(vm: &VirtualMachine, msg: String) -> PyBaseExceptionRef {
646+
pub fn new_struct_error(vm: &VirtualMachine, msg: impl Into<String>) -> PyBaseExceptionRef {
651647
// can't just STRUCT_ERROR.get().unwrap() cause this could be called before from buffer
652648
// machinery, independent of whether _struct was ever imported
653-
vm.new_exception_msg(struct_error_type(vm).clone(), msg)
649+
vm.new_exception_msg(struct_error_type(vm).clone(), msg.into())
654650
}

‎vm/src/builtins/classmethod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl Representable for PyClassMethod {
184184
.map(|n| n.as_str()),
185185
class.module(vm).downcast_ref::<PyStr>().map(|m| m.as_str()),
186186
) {
187-
(None, _) => return Err(vm.new_type_error("Unknown qualified name".into())),
187+
(None, _) => return Err(vm.new_type_error("Unknown qualified name")),
188188
(Some(qualname), Some(module)) if module != "builtins" => {
189189
format!("<{module}.{qualname}({callable})>")
190190
}

‎vm/src/builtins/module.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ impl Representable for PyModule {
208208
let module_repr = importlib.get_attr("_module_repr", vm)?;
209209
let repr = module_repr.call((zelf.to_owned(),), vm)?;
210210
repr.downcast()
211-
.map_err(|_| vm.new_type_error("_module_repr did not return a string".into()))
211+
.map_err(|_| vm.new_type_error("_module_repr did not return a string"))
212212
}
213213

214214
#[cold]

‎vm/src/builtins/object.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ impl PyBaseObject {
350350
.map(|n| n.as_str()),
351351
class.module(vm).downcast_ref::<PyStr>().map(|m| m.as_str()),
352352
) {
353-
(None, _) => Err(vm.new_type_error("Unknown qualified name".into())),
353+
(None, _) => Err(vm.new_type_error("Unknown qualified name")),
354354
(Some(qualname), Some(module)) if module != "builtins" => Ok(PyStr::from(format!(
355355
"<{}.{} object at {:#x}>",
356356
module,

‎vm/src/builtins/staticmethod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl Representable for PyStaticMethod {
153153
.map(|n| n.as_str()),
154154
class.module(vm).downcast_ref::<PyStr>().map(|m| m.as_str()),
155155
) {
156-
(None, _) => Err(vm.new_type_error("Unknown qualified name".into())),
156+
(None, _) => Err(vm.new_type_error("Unknown qualified name")),
157157
(Some(qualname), Some(module)) if module != "builtins" => {
158158
Ok(format!("<{module}.{qualname}({callable})>"))
159159
}

‎vm/src/exceptions.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,7 @@ impl ExceptionCtor {
428428
match (self, exc_inst) {
429429
// both are instances; which would we choose?
430430
(Self::Instance(_exc_a), Some(_exc_b)) => {
431-
Err(vm
432-
.new_type_error("instance exception may not have a separate value".to_owned()))
431+
Err(vm.new_type_error("instance exception may not have a separate value"))
433432
}
434433
// if the "type" is an instance and the value isn't, use the "type"
435434
(Self::Instance(exc), None) => Ok(exc),
@@ -672,7 +671,7 @@ impl PyRef<PyBaseException> {
672671
if !vm.is_none(&state) {
673672
let dict = state
674673
.downcast::<crate::builtins::PyDict>()
675-
.map_err(|_| vm.new_type_error("state is not a dictionary".to_owned()))?;
674+
.map_err(|_| vm.new_type_error("state is not a dictionary"))?;
676675

677676
for (key, value) in &dict {
678677
let key_str = key.str(vm)?;
@@ -691,7 +690,7 @@ impl Constructor for PyBaseException {
691690

692691
fn py_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult {
693692
if cls.is(PyBaseException::class(&vm.ctx)) && !args.kwargs.is_empty() {
694-
return Err(vm.new_type_error("BaseException() takes no keyword arguments".to_owned()));
693+
return Err(vm.new_type_error("BaseException() takes no keyword arguments"));
695694
}
696695
PyBaseException::new(args.args, vm)
697696
.into_ref_with_type(vm, cls)
@@ -1149,7 +1148,7 @@ impl serde::Serialize for SerializeException<'_, '_> {
11491148
}
11501149

11511150
pub fn cstring_error(vm: &VirtualMachine) -> PyBaseExceptionRef {
1152-
vm.new_value_error("embedded null character".to_owned())
1151+
vm.new_value_error("embedded null character")
11531152
}
11541153

11551154
impl ToPyException for std::ffi::NulError {
@@ -1353,8 +1352,7 @@ pub(super) mod types {
13531352
// Check for any remaining invalid keyword arguments
13541353
if let Some(invalid_key) = kwargs.keys().next() {
13551354
return Err(vm.new_type_error(format!(
1356-
"'{}' is an invalid keyword argument for ImportError",
1357-
invalid_key
1355+
"'{invalid_key}' is an invalid keyword argument for ImportError"
13581356
)));
13591357
}
13601358

‎vm/src/format.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ impl IntoPyException for FormatSpecError {
2828
vm.new_value_error(msg)
2929
}
3030
FormatSpecError::PrecisionNotAllowed => {
31-
vm.new_value_error("Precision not allowed in integer format specifier".to_owned())
31+
vm.new_value_error("Precision not allowed in integer format specifier")
3232
}
3333
FormatSpecError::NotAllowed(s) => {
3434
let msg = format!("{s} not allowed with integer format specifier 'c'");
3535
vm.new_value_error(msg)
3636
}
3737
FormatSpecError::UnableToConvert => {
38-
vm.new_value_error("Unable to convert int to float".to_owned())
38+
vm.new_value_error("Unable to convert int to float")
3939
}
4040
FormatSpecError::CodeNotInRange => {
41-
vm.new_overflow_error("%c arg not in range(0x110000)".to_owned())
41+
vm.new_overflow_error("%c arg not in range(0x110000)")
4242
}
4343
FormatSpecError::NotImplemented(c, s) => {
4444
let msg = format!("Format code '{c}' for object of type '{s}' not implemented yet");
@@ -52,9 +52,9 @@ impl ToPyException for FormatParseError {
5252
fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef {
5353
match self {
5454
FormatParseError::UnmatchedBracket => {
55-
vm.new_value_error("expected '}' before end of string".to_owned())
55+
vm.new_value_error("expected '}' before end of string")
5656
}
57-
_ => vm.new_value_error("Unexpected error parsing format string".to_owned()),
57+
_ => vm.new_value_error("Unexpected error parsing format string"),
5858
}
5959
}
6060
}
@@ -130,30 +130,28 @@ pub(crate) fn format(
130130
FieldType::Auto => {
131131
if seen_index {
132132
return Err(vm.new_value_error(
133-
"cannot switch from manual field specification to automatic field numbering"
134-
.to_owned(),
133+
"cannot switch from manual field specification to automatic field numbering",
135134
));
136135
}
137136
auto_argument_index += 1;
138137
arguments
139138
.args
140139
.get(auto_argument_index - 1)
141140
.cloned()
142-
.ok_or_else(|| vm.new_index_error("tuple index out of range".to_owned()))
141+
.ok_or_else(|| vm.new_index_error("tuple index out of range"))
143142
}
144143
FieldType::Index(index) => {
145144
if auto_argument_index != 0 {
146145
return Err(vm.new_value_error(
147-
"cannot switch from automatic field numbering to manual field specification"
148-
.to_owned(),
146+
"cannot switch from automatic field numbering to manual field specification",
149147
));
150148
}
151149
seen_index = true;
152150
arguments
153151
.args
154152
.get(index)
155153
.cloned()
156-
.ok_or_else(|| vm.new_index_error("tuple index out of range".to_owned()))
154+
.ok_or_else(|| vm.new_index_error("tuple index out of range"))
157155
}
158156
FieldType::Keyword(keyword) => keyword
159157
.as_str()
@@ -170,7 +168,7 @@ pub(crate) fn format_map(
170168
) -> PyResult<Wtf8Buf> {
171169
format_internal(vm, format, &mut |field_type| match field_type {
172170
FieldType::Auto | FieldType::Index(_) => {
173-
Err(vm.new_value_error("Format string contains positional fields".to_owned()))
171+
Err(vm.new_value_error("Format string contains positional fields"))
174172
}
175173
FieldType::Keyword(keyword) => dict.get_item(&keyword, vm),
176174
})

‎vm/src/sequence.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ where
101101
let n = vm.check_repeat_or_overflow_error(self.as_ref().len(), n)?;
102102

103103
if n > 1 && std::mem::size_of_val(self.as_ref()) >= MAX_MEMORY_SIZE / n {
104-
return Err(vm.new_memory_error("".to_owned()));
104+
return Err(vm.new_memory_error(""));
105105
}
106106

107107
let mut v = Vec::with_capacity(n * self.as_ref().len());

0 commit comments

Comments
 (0)