Skip to content

Commit b77ada9

Browse files
ShaharNavehShaharNaveh
authored
Code nitpicks (RustPython#8058)
* Code nitpicks * fix error message * Use `vm.new_.*_error` methods
1 parent 7f75ef1 commit b77ada9

19 files changed

Lines changed: 291 additions & 305 deletions

File tree

‎crates/stdlib/src/array.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -882,10 +882,7 @@ pub mod array {
882882
self._from_bytes(b.as_bytes(), itemsize, vm)?;
883883

884884
if not_enough_bytes {
885-
Err(vm.new_exception_msg(
886-
vm.ctx.exceptions.eof_error.to_owned(),
887-
"read() didn't return enough bytes".into(),
888-
))
885+
Err(vm.new_eof_error("read() didn't return enough bytes"))
889886
} else {
890887
Ok(())
891888
}

‎crates/stdlib/src/multiprocessing.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,8 @@ mod _multiprocessing {
175175
fn release(&self, vm: &VirtualMachine) -> PyResult<()> {
176176
if self.kind == RECURSIVE_MUTEX {
177177
if !ismine!(self) {
178-
return Err(vm.new_exception_msg(
179-
vm.ctx.exceptions.assertion_error.to_owned(),
180-
"attempt to release recursive lock not owned by thread".into(),
178+
return Err(vm.new_assertion_error(
179+
"attempt to release recursive lock not owned by thread",
181180
));
182181
}
183182
if self.count.load(Ordering::Acquire) > 1 {
@@ -597,9 +596,8 @@ mod _multiprocessing {
597596
if self.kind == RECURSIVE_MUTEX {
598597
// if (!ISMINE(self))
599598
if !ismine!(self) {
600-
return Err(vm.new_exception_msg(
601-
vm.ctx.exceptions.assertion_error.to_owned(),
602-
"attempt to release recursive lock not owned by thread".into(),
599+
return Err(vm.new_assertion_error(
600+
"attempt to release recursive lock not owned by thread",
603601
));
604602
}
605603
// if (self->count > 1) { --self->count; Py_RETURN_NONE; }

‎crates/stdlib/src/ssl.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4657,10 +4657,7 @@ mod _ssl {
46574657
// It's a memoryview, check if contiguous
46584658
let is_contiguous: bool = mem_view.try_to_bool(vm)?;
46594659
if !is_contiguous {
4660-
return Err(vm.new_exception_msg(
4661-
vm.ctx.exceptions.buffer_error.to_owned(),
4662-
"non-contiguous buffer is not supported".into(),
4663-
));
4660+
return Err(vm.new_buffer_error("non-contiguous buffer is not supported"));
46644661
}
46654662
}
46664663

‎crates/vm/src/builtins/int.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,7 @@ fn inner_truediv(i1: &BigInt, i2: &BigInt, vm: &VirtualMachine) -> PyResult {
236236
let float = true_div(i1, i2);
237237

238238
if float.is_infinite() {
239-
Err(vm.new_exception_msg(
240-
vm.ctx.exceptions.overflow_error.to_owned(),
241-
"integer division result too large for a float".into(),
242-
))
239+
Err(vm.new_overflow_error("integer division result too large for a float"))
243240
} else {
244241
Ok(vm.ctx.new_float(float).into())
245242
}

‎crates/vm/src/builtins/interpolation.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ impl PyInterpolation {
4646
.downcast_ref::<PyStr>()
4747
.is_some_and(|s| matches!(s.to_str(), Some("s" | "r" | "a")));
4848
if !is_valid {
49-
return Err(vm.new_exception_msg(
50-
vm.ctx.exceptions.system_error.to_owned(),
51-
"Interpolation() argument 'conversion' must be one of 's', 'a' or 'r'".into(),
49+
return Err(vm.new_system_error(
50+
"Interpolation() argument 'conversion' must be one of 's', 'a' or 'r'",
5251
));
5352
}
5453
Ok(Self {

‎crates/vm/src/frame.rs‎

Lines changed: 34 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2038,10 +2038,9 @@ impl ExecutingFrame<'_> {
20382038
} else {
20392039
// Both merged cells (LOCAL|CELL) and non-merged cells get unbound local error
20402040
let name = self.localsplus_name(localsplus_idx);
2041-
vm.new_exception_msg(
2042-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2043-
format!("local variable '{name}' referenced before assignment").into(),
2044-
)
2041+
vm.new_unbound_local_error(format!(
2042+
"local variable '{name}' referenced before assignment"
2043+
))
20452044
}
20462045
}
20472046

@@ -2378,14 +2377,10 @@ impl ExecutingFrame<'_> {
23782377
let fastlocals = self.localsplus.fastlocals_mut();
23792378
let idx = var_num.get(arg);
23802379
if fastlocals[idx].is_none() {
2381-
return Err(vm.new_exception_msg(
2382-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2383-
format!(
2384-
"local variable '{}' referenced before assignment",
2385-
self.code.varnames[idx]
2386-
)
2387-
.into(),
2388-
));
2380+
return Err(vm.new_unbound_local_error(format!(
2381+
"local variable '{}' referenced before assignment",
2382+
self.code.varnames[idx]
2383+
)));
23892384
}
23902385
fastlocals[idx] = None;
23912386
Ok(None)
@@ -2884,10 +2879,9 @@ impl ExecutingFrame<'_> {
28842879
varname: &'static PyStrInterned,
28852880
vm: &VirtualMachine,
28862881
) -> PyBaseExceptionRef {
2887-
vm.new_exception_msg(
2888-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2889-
format!("local variable '{varname}' referenced before assignment").into(),
2890-
)
2882+
vm.new_unbound_local_error(format!(
2883+
"local variable '{varname}' referenced before assignment"
2884+
))
28912885
}
28922886
let idx = var_num.get(arg);
28932887
let x = self.localsplus.fastlocals()[idx]
@@ -2910,14 +2904,10 @@ impl ExecutingFrame<'_> {
29102904
// (LoadFast in RustPython already does this check)
29112905
let idx = var_num.get(arg);
29122906
let x = self.localsplus.fastlocals()[idx].clone().ok_or_else(|| {
2913-
vm.new_exception_msg(
2914-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2915-
format!(
2916-
"local variable '{}' referenced before assignment",
2917-
self.code.varnames[idx]
2918-
)
2919-
.into(),
2920-
)
2907+
vm.new_unbound_local_error(format!(
2908+
"local variable '{}' referenced before assignment",
2909+
self.code.varnames[idx]
2910+
))
29212911
})?;
29222912
self.push_value(x);
29232913
Ok(None)
@@ -2929,24 +2919,16 @@ impl ExecutingFrame<'_> {
29292919
let (idx1, idx2) = oparg.indexes();
29302920
let fastlocals = self.localsplus.fastlocals();
29312921
let x1 = fastlocals[idx1].clone().ok_or_else(|| {
2932-
vm.new_exception_msg(
2933-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2934-
format!(
2935-
"local variable '{}' referenced before assignment",
2936-
self.code.varnames[idx1]
2937-
)
2938-
.into(),
2939-
)
2922+
vm.new_unbound_local_error(format!(
2923+
"local variable '{}' referenced before assignment",
2924+
self.code.varnames[idx1]
2925+
))
29402926
})?;
29412927
let x2 = fastlocals[idx2].clone().ok_or_else(|| {
2942-
vm.new_exception_msg(
2943-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2944-
format!(
2945-
"local variable '{}' referenced before assignment",
2946-
self.code.varnames[idx2]
2947-
)
2948-
.into(),
2949-
)
2928+
vm.new_unbound_local_error(format!(
2929+
"local variable '{}' referenced before assignment",
2930+
self.code.varnames[idx2]
2931+
))
29502932
})?;
29512933
self.push_value(x1);
29522934
self.push_value(x2);
@@ -2958,14 +2940,10 @@ impl ExecutingFrame<'_> {
29582940
Instruction::LoadFastBorrow { var_num } => {
29592941
let idx = var_num.get(arg);
29602942
let x = self.localsplus.fastlocals()[idx].clone().ok_or_else(|| {
2961-
vm.new_exception_msg(
2962-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2963-
format!(
2964-
"local variable '{}' referenced before assignment",
2965-
self.code.varnames[idx]
2966-
)
2967-
.into(),
2968-
)
2943+
vm.new_unbound_local_error(format!(
2944+
"local variable '{}' referenced before assignment",
2945+
self.code.varnames[idx]
2946+
))
29692947
})?;
29702948
self.push_value(x);
29712949
Ok(None)
@@ -2975,24 +2953,16 @@ impl ExecutingFrame<'_> {
29752953
let (idx1, idx2) = oparg.indexes();
29762954
let fastlocals = self.localsplus.fastlocals();
29772955
let x1 = fastlocals[idx1].clone().ok_or_else(|| {
2978-
vm.new_exception_msg(
2979-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2980-
format!(
2981-
"local variable '{}' referenced before assignment",
2982-
self.code.varnames[idx1]
2983-
)
2984-
.into(),
2985-
)
2956+
vm.new_unbound_local_error(format!(
2957+
"local variable '{}' referenced before assignment",
2958+
self.code.varnames[idx1]
2959+
))
29862960
})?;
29872961
let x2 = fastlocals[idx2].clone().ok_or_else(|| {
2988-
vm.new_exception_msg(
2989-
vm.ctx.exceptions.unbound_local_error.to_owned(),
2990-
format!(
2991-
"local variable '{}' referenced before assignment",
2992-
self.code.varnames[idx2]
2993-
)
2994-
.into(),
2995-
)
2962+
vm.new_unbound_local_error(format!(
2963+
"local variable '{}' referenced before assignment",
2964+
self.code.varnames[idx2]
2965+
))
29962966
})?;
29972967
self.push_value(x1);
29982968
self.push_value(x2);

‎crates/vm/src/protocol/buffer.rs‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,10 @@ pub struct PyBuffer {
4444
impl PyBuffer {
4545
#[must_use]
4646
pub fn new(obj: PyObjectRef, desc: BufferDescriptor, methods: &'static BufferMethods) -> Self {
47-
let zelf = Self {
48-
obj,
49-
desc: desc.validate(),
50-
methods,
51-
};
47+
#[cfg(debug_assertions)]
48+
let desc = desc.validate();
49+
50+
let zelf = Self { obj, desc, methods };
5251
zelf.retain();
5352
zelf
5453
}
@@ -216,30 +215,25 @@ impl BufferDescriptor {
216215
if self.ndim() == 0 {
217216
// Empty structures (len=0) can have itemsize=0
218217
if self.len > 0 {
219-
assert!(self.itemsize != 0);
218+
debug_assert_ne!(self.itemsize, 0);
220219
}
221-
assert!(self.itemsize == self.len);
220+
debug_assert_eq!(self.itemsize, self.len);
222221
} else {
223222
let mut shape_product = 1;
224223
let has_zero_dim = self.dim_desc.iter().any(|(s, _, _)| *s == 0);
225224
for (shape, stride, suboffset) in self.dim_desc.iter().copied() {
226225
shape_product *= shape;
227-
assert!(suboffset >= 0);
226+
debug_assert!(suboffset >= 0);
228227
// For empty arrays (any dimension is 0), strides can be 0
229228
if !has_zero_dim {
230-
assert!(stride != 0);
229+
debug_assert_ne!(stride, 0);
231230
}
232231
}
233-
assert!(shape_product * self.itemsize == self.len);
232+
debug_assert_eq!(shape_product * self.itemsize, self.len);
234233
}
235234
self
236235
}
237236

238-
#[cfg(not(debug_assertions))]
239-
pub fn validate(self) -> Self {
240-
self
241-
}
242-
243237
#[must_use]
244238
pub fn ndim(&self) -> usize {
245239
self.dim_desc.len()
@@ -396,6 +390,7 @@ impl BufferDescriptor {
396390
}
397391
}
398392

393+
#[must_use]
399394
fn is_last_dim_contiguous(&self) -> bool {
400395
let (_, stride, suboffset) = self.dim_desc[self.ndim() - 1];
401396
suboffset == 0 && stride == self.itemsize as isize

‎crates/vm/src/protocol/callable.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ use crate::{
77

88
impl PyObject {
99
#[inline]
10+
#[must_use]
1011
pub fn to_callable(&self) -> Option<PyCallable<'_>> {
1112
PyCallable::new(self)
1213
}
1314

1415
#[inline]
16+
#[must_use]
1517
pub fn is_callable(&self) -> bool {
1618
self.to_callable().is_some()
1719
}
@@ -134,6 +136,7 @@ impl<'a> PyCallable<'a> {
134136
}
135137

136138
/// Trace events for sys.settrace and sys.setprofile.
139+
#[derive(Clone, Copy, Eq, PartialEq)]
137140
pub(crate) enum TraceEvent {
138141
Call,
139142
Return,
@@ -147,7 +150,8 @@ pub(crate) enum TraceEvent {
147150

148151
impl TraceEvent {
149152
/// Whether sys.settrace receives this event.
150-
fn is_trace_event(&self) -> bool {
153+
#[must_use]
154+
const fn is_trace_event(&self) -> bool {
151155
matches!(
152156
self,
153157
Self::Call | Self::Return | Self::Exception | Self::Line | Self::Opcode
@@ -157,15 +161,17 @@ impl TraceEvent {
157161
/// Whether sys.setprofile receives this event.
158162
/// In legacy_tracing.c, profile callbacks are only registered for
159163
/// PY_RETURN, PY_UNWIND, C_CALL, C_RETURN, C_RAISE.
160-
fn is_profile_event(&self) -> bool {
164+
#[must_use]
165+
const fn is_profile_event(&self) -> bool {
161166
matches!(
162167
self,
163168
Self::Call | Self::Return | Self::CCall | Self::CReturn | Self::CException
164169
)
165170
}
166171

167172
/// Whether this event is dispatched only when f_trace_opcodes is set.
168-
pub(crate) fn is_opcode_event(&self) -> bool {
173+
#[must_use]
174+
pub(crate) const fn is_opcode_event(&self) -> bool {
169175
matches!(self, Self::Opcode)
170176
}
171177
}

‎crates/vm/src/protocol/iter.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ use crate::{
77
use core::borrow::Borrow;
88
use core::ops::Deref;
99

10-
/// Iterator Protocol
11-
// https://docs.python.org/3/c-api/iter.html
10+
/// [Iterator Protocol](https://docs.python.org/3/c-api/iter.html).
1211
#[derive(Debug, Clone)]
1312
#[repr(transparent)]
1413
pub struct PyIter<O = PyObjectRef>(O)
@@ -31,9 +30,11 @@ impl<O> PyIter<O>
3130
where
3231
O: Borrow<PyObject>,
3332
{
33+
#[must_use]
3434
pub const fn new(obj: O) -> Self {
3535
Self(obj)
3636
}
37+
3738
pub fn next(&self, vm: &VirtualMachine) -> PyResult<PyIterReturn> {
3839
let iternext = self
3940
.0
@@ -193,7 +194,7 @@ impl PyIterReturn {
193194
match self {
194195
Self::Return(obj) => Ok(obj),
195196
Self::StopIteration(v) => Err({
196-
let args = if let Some(v) = v { vec![v] } else { Vec::new() };
197+
let args = v.map_or_else(Vec::new, |v| vec![v]);
197198
vm.new_exception(vm.ctx.exceptions.stop_async_iteration.to_owned(), args)
198199
}),
199200
}

0 commit comments

Comments
 (0)