Skip to content

Commit 67e66bd

Browse files
ShaharNavehShaharNaveh
authored
code nits (RustPython#7908)
* Slight cleanup of super.rs * frame.rs * buffer.rs * bool.rs
1 parent e16f1aa commit 67e66bd

8 files changed

Lines changed: 117 additions & 81 deletions

File tree

‎crates/vm/src/buffer.rs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type UnpackFunc = fn(&VirtualMachine, &[u8]) -> PyObjectRef;
1818

1919
static OVERFLOW_MSG: &str = "total struct size too long"; // not a const to reduce code size
2020

21-
#[derive(Debug, Copy, Clone, PartialEq)]
21+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
2222
pub(crate) enum Endianness {
2323
Native,
2424
Little,
@@ -40,21 +40,30 @@ impl Endianness {
4040
Some(b'>' | b'!') => Self::Big,
4141
_ => return Self::Native,
4242
};
43-
chars.next().unwrap();
43+
44+
// SAFETY:
45+
// We just ensured with `chars.peek()` that this is safe
46+
unsafe {
47+
let _ = chars.next().unwrap_unchecked();
48+
}
4449
e
4550
}
4651
}
4752

4853
trait ByteOrder {
4954
fn convert<I: PrimInt>(i: I) -> I;
5055
}
56+
5157
enum BigEndian {}
58+
5259
impl ByteOrder for BigEndian {
5360
fn convert<I: PrimInt>(i: I) -> I {
5461
i.to_be()
5562
}
5663
}
64+
5765
enum LittleEndian {}
66+
5867
impl ByteOrder for LittleEndian {
5968
fn convert<I: PrimInt>(i: I) -> I {
6069
i.to_le()
@@ -66,7 +75,7 @@ type NativeEndian = cfg_select! {
6675
target_endian = "little" => LittleEndian,
6776
};
6877

69-
#[derive(Copy, Clone, num_enum::TryFromPrimitive)]
78+
#[derive(Copy, Clone, num_enum::TryFromPrimitive, Eq, PartialEq)]
7079
#[repr(u8)]
7180
pub(crate) enum FormatType {
7281
Pad = b'x',
@@ -105,6 +114,7 @@ impl FormatType {
105114
fn info(self, e: Endianness) -> &'static FormatInfo {
106115
use FormatType::*;
107116
use mem::{align_of, size_of};
117+
108118
macro_rules! native_info {
109119
($t:ty) => {{
110120
&FormatInfo {
@@ -115,6 +125,7 @@ impl FormatType {
115125
}
116126
}};
117127
}
128+
118129
macro_rules! nonnative_info {
119130
($t:ty, $end:ty) => {{
120131
&FormatInfo {
@@ -125,6 +136,7 @@ impl FormatType {
125136
}
126137
}};
127138
}
139+
128140
macro_rules! match_nonnative {
129141
($zelf:expr, $end:ty) => {{
130142
match $zelf {
@@ -158,6 +170,7 @@ impl FormatType {
158170
}
159171
}};
160172
}
173+
161174
match e {
162175
Endianness::Native => match self {
163176
Pad | Str | Pascal => &FormatInfo {
@@ -381,6 +394,7 @@ pub(crate) struct FormatInfo {
381394
pub pack: Option<PackFunc>,
382395
pub unpack: Option<UnpackFunc>,
383396
}
397+
384398
impl fmt::Debug for FormatInfo {
385399
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
386400
f.debug_struct("FormatInfo")

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ impl PyObjectRef {
3737
pub fn try_to_bool(self, vm: &VirtualMachine) -> PyResult<bool> {
3838
if self.is(&vm.ctx.true_value) {
3939
return Ok(true);
40-
}
41-
if self.is(&vm.ctx.false_value) {
40+
} else if self.is(&vm.ctx.false_value) {
4241
return Ok(false);
4342
}
4443

@@ -83,10 +82,9 @@ impl Constructor for PyBool {
8382
fn slot_new(zelf: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult {
8483
let x: Self::Args = args.bind(vm)?;
8584
if !zelf.fast_isinstance(vm.ctx.types.type_type) {
86-
let actual_class = zelf.class();
87-
let actual_type = &actual_class.name();
8885
return Err(vm.new_type_error(format!(
89-
"requires a 'type' object but received a '{actual_type}'"
86+
"requires a 'type' object but received a '{}'",
87+
zelf.class().name()
9088
)));
9189
}
9290
let val = x.map_or(Ok(false), |val| val.try_to_bool(vm))?;

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

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,40 +43,40 @@ pub(crate) mod stack_analysis {
4343
}
4444

4545
impl Kind {
46-
fn from_i64(v: i64) -> Option<Self> {
47-
match v {
48-
1 => Some(Self::Iterator),
49-
2 => Some(Self::Except),
50-
3 => Some(Self::Object),
51-
4 => Some(Self::Null),
52-
5 => Some(Self::Lasti),
53-
_ => None,
54-
}
46+
const fn from_i64(v: i64) -> Option<Self> {
47+
Some(match v {
48+
1 => Self::Iterator,
49+
2 => Self::Except,
50+
3 => Self::Object,
51+
4 => Self::Null,
52+
5 => Self::Lasti,
53+
_ => return None,
54+
})
5555
}
5656
}
5757

58-
pub(crate) fn push_value(stack: i64, kind: i64) -> i64 {
58+
pub(crate) const fn push_value(stack: i64, kind: i64) -> i64 {
5959
if (stack as u64) >= WILL_OVERFLOW {
6060
OVERFLOWED
6161
} else {
6262
(stack << BITS_PER_BLOCK) | kind
6363
}
6464
}
6565

66-
pub(crate) fn pop_value(stack: i64) -> i64 {
66+
pub(crate) const fn pop_value(stack: i64) -> i64 {
6767
stack >> BITS_PER_BLOCK
6868
}
6969

70-
pub(crate) fn top_of_stack(stack: i64) -> i64 {
70+
pub(crate) const fn top_of_stack(stack: i64) -> i64 {
7171
stack & MASK
7272
}
7373

74-
fn peek(stack: i64, n: u32) -> i64 {
74+
const fn peek(stack: i64, n: u32) -> i64 {
7575
debug_assert!(n >= 1);
7676
(stack >> (BITS_PER_BLOCK * (n - 1))) & MASK
7777
}
7878

79-
fn stack_swap(stack: i64, n: u32) -> i64 {
79+
const fn stack_swap(stack: i64, n: u32) -> i64 {
8080
debug_assert!(n >= 1);
8181
let to_swap = peek(stack, n);
8282
let top = top_of_stack(stack);
@@ -85,7 +85,7 @@ pub(crate) mod stack_analysis {
8585
(replaced_low & !MASK) | to_swap
8686
}
8787

88-
fn pop_to_level(mut stack: i64, level: u32) -> i64 {
88+
const fn pop_to_level(mut stack: i64, level: u32) -> i64 {
8989
if level == 0 {
9090
return EMPTY_STACK;
9191
}
@@ -97,20 +97,21 @@ pub(crate) mod stack_analysis {
9797
stack
9898
}
9999

100-
fn compatible_kind(from: i64, to: i64) -> bool {
100+
#[must_use]
101+
const fn compatible_kind(from: i64, to: i64) -> bool {
101102
if to == 0 {
102-
return false;
103-
}
104-
if to == Kind::Object as i64 {
105-
return from != Kind::Null as i64;
106-
}
107-
if to == Kind::Null as i64 {
108-
return true;
103+
false
104+
} else if to == Kind::Object as i64 {
105+
from != Kind::Null as i64
106+
} else if to == Kind::Null as i64 {
107+
true
108+
} else {
109+
from == to
109110
}
110-
from == to
111111
}
112112

113-
pub(crate) fn compatible_stack(from_stack: i64, to_stack: i64) -> bool {
113+
#[must_use]
114+
pub(crate) const fn compatible_stack(from_stack: i64, to_stack: i64) -> bool {
114115
if from_stack < 0 || to_stack < 0 {
115116
return false;
116117
}
@@ -131,14 +132,17 @@ pub(crate) mod stack_analysis {
131132
to == 0
132133
}
133134

134-
pub(crate) fn explain_incompatible_stack(to_stack: i64) -> &'static str {
135+
pub(crate) const fn explain_incompatible_stack(to_stack: i64) -> &'static str {
135136
debug_assert!(to_stack != 0);
137+
136138
if to_stack == OVERFLOWED {
137139
return "stack is too deep to analyze";
138140
}
141+
139142
if to_stack == UNINITIALIZED {
140143
return "can't jump into an exception handler, or code may be unreachable";
141144
}
145+
142146
match Kind::from_i64(top_of_stack(to_stack)) {
143147
Some(Kind::Except) => "can't jump into an 'except' block as there's no exception",
144148
Some(Kind::Lasti) => "can't jump into a re-raising block as there's no location",

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,26 @@ fn format_missing_args(
3333
missing: &mut Vec<impl core::fmt::Display>,
3434
) -> String {
3535
let count = missing.len();
36+
3637
let last = if missing.len() > 1 {
3738
missing.pop()
3839
} else {
3940
None
4041
};
42+
4143
let (and, right): (&str, String) = if let Some(last) = last {
4244
(
4345
if missing.len() == 1 {
4446
"' and '"
4547
} else {
4648
"', and '"
4749
},
48-
format!("{last}"),
50+
last.to_string(),
4951
)
5052
} else {
5153
("", String::new())
5254
};
55+
5356
format!(
5457
"{qualname}() missing {count} required {kind} argument{}: '{}{}{right}'",
5558
if count == 1 { "" } else { "s" },
@@ -1268,12 +1271,12 @@ impl PyBoundMethod {
12681271
}
12691272

12701273
#[inline]
1271-
pub(crate) fn function_obj(&self) -> &PyObjectRef {
1274+
pub(crate) const fn function_obj(&self) -> &PyObjectRef {
12721275
&self.function
12731276
}
12741277

12751278
#[inline]
1276-
pub(crate) fn self_obj(&self) -> &PyObjectRef {
1279+
pub(crate) const fn self_obj(&self) -> &PyObjectRef {
12771280
&self.object
12781281
}
12791282

@@ -1398,6 +1401,7 @@ impl Representable for PyBoundMethod {
13981401
pub(crate) struct PyCell {
13991402
contents: PyMutex<Option<PyObjectRef>>,
14001403
}
1404+
14011405
pub(crate) type PyCellRef = PyRef<PyCell>;
14021406

14031407
impl PyPayload for PyCell {
@@ -1426,6 +1430,7 @@ impl PyCell {
14261430
pub(crate) fn get(&self) -> Option<PyObjectRef> {
14271431
self.contents.lock().clone()
14281432
}
1433+
14291434
pub(crate) fn set(&self, x: Option<PyObjectRef>) {
14301435
*self.contents.lock() = x;
14311436
}
@@ -1435,6 +1440,7 @@ impl PyCell {
14351440
self.get()
14361441
.ok_or_else(|| vm.new_value_error("Cell is empty"))
14371442
}
1443+
14381444
#[pygetset(setter)]
14391445
fn set_cell_contents(&self, x: PySetterValue) {
14401446
match x {
@@ -1488,6 +1494,7 @@ pub(crate) fn vectorcall_function(
14881494
args.truncate(nargs);
14891495
FuncArgs::from(args)
14901496
};
1497+
14911498
zelf.invoke(func_args, vm)
14921499
}
14931500

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

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ impl Initializer for PySuper {
8686
if frame.code.arg_count == 0 {
8787
return Err(vm.new_runtime_error("super(): no arguments"));
8888
}
89+
8990
// SAFETY: Frame is current and not concurrently mutated.
9091
use rustpython_compiler_core::bytecode::CO_FAST_CELL;
9192
let obj = unsafe { frame.fastlocals() }[0]
@@ -165,9 +166,9 @@ impl GetAttr for PySuper {
165166
Some(o) => o.clone(),
166167
None => return skip(zelf, name),
167168
};
169+
168170
// We want __class__ to return the class of the super object
169171
// (i.e. super, or a subclass), not the class of su->obj.
170-
171172
if name.as_bytes() == b"__class__" {
172173
return skip(zelf, name);
173174
}
@@ -280,21 +281,23 @@ pub(crate) fn init(context: &'static Context) {
280281
let super_type = &context.types.super_type;
281282
PySuper::extend_class(context, super_type);
282283

283-
let super_doc = "super() -> same as super(__class__, <first argument>)\n\
284-
super(type) -> unbound super object\n\
285-
super(type, obj) -> bound super object; requires isinstance(obj, type)\n\
286-
super(type, type2) -> bound super object; requires issubclass(type2, type)\n\
287-
Typical use to call a cooperative superclass method:\n\
288-
class C(B):\n \
289-
def meth(self, arg):\n \
290-
super().meth(arg)\n\
291-
This works for class methods too:\n\
292-
class C(B):\n \
293-
@classmethod\n \
294-
def cmeth(cls, arg):\n \
295-
super().cmeth(arg)\n";
284+
const SUPER_DOC: &str = "\
285+
super() -> same as super(__class__, <first argument>)
286+
super(type) -> unbound super object
287+
super(type, obj) -> bound super object; requires isinstance(obj, type)
288+
super(type, type2) -> bound super object; requires issubclass(type2, type)
289+
Typical use to call a cooperative superclass method:
290+
class C(B):
291+
def meth(self, arg):
292+
super().meth(arg)
293+
This works for class methods too:
294+
class C(B):
295+
@classmethod
296+
def cmeth(cls, arg):
297+
super().cmeth(arg)
298+
";
296299

297300
extend_class!(context, super_type, {
298-
"__doc__" => context.new_str(super_doc),
301+
"__doc__" => context.new_str(SUPER_DOC),
299302
});
300303
}

0 commit comments

Comments
 (0)