Skip to content

Commit 185d167

Browse files
discord9youknowone
discord9
authored and
youknowone
committed
refactor: accord to Code Review
1 parent 05da008 commit 185d167

File tree

7 files changed

+138
-144
lines changed

7 files changed

+138
-144
lines changed

‎Lib/test/test_ordered_dict.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,6 @@ class A:
651651
gc.collect()
652652
# TODO: RustPython, Need to fix this: somehow after del A, it takes two call to `gc.collect()`
653653
# for gc to realize a loop is there and to be collected
654-
gc.collect()
655654
self.assertIsNone(r())
656655

657656
# TODO: RUSTPYTHON

‎Lib/test/test_sys_setprofile.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,7 @@ def trace_call(self, frame):
6969

7070
def trace_return(self, frame):
7171
self.add_event('return', frame)
72-
if len(self.stack)!=0:
73-
self.stack.pop()
72+
self.stack.pop()
7473

7574
def trace_exception(self, frame):
7675
self.testcase.fail(

‎derive-impl/src/pyclass.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,9 @@ pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStrea
345345
if is_trace {
346346
quote! {
347347
#[cfg(feature = "gc_bacon")]
348-
impl ::rustpython_vm::object::MaybeTrace for #ident{
349-
const IS_TRACE: bool=true;
350-
fn try_trace(&self, tracer_fn: &mut ::rustpython_vm::object::TracerFn){
348+
impl ::rustpython_vm::object::MaybeTrace for #ident {
349+
const IS_TRACE: bool = true;
350+
fn try_trace(&self, tracer_fn: &mut ::rustpython_vm::object::TracerFn) {
351351
::rustpython_vm::object::Trace::trace(self, tracer_fn);
352352
}
353353
}
@@ -357,7 +357,7 @@ pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStrea
357357
// #attrs
358358
quote! {
359359
#[cfg(feature = "gc_bacon")]
360-
impl ::rustpython_vm::object::MaybeTrace for #ident{}
360+
impl ::rustpython_vm::object::MaybeTrace for #ident { }
361361
}
362362
}
363363
};

‎vm/src/frame.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,15 +349,15 @@ impl ExecutingFrame<'_> {
349349
// Execute until return or exception:
350350
let instrs = &self.code.instructions;
351351
let mut arg_state = bytecode::OpArgState::default();
352-
let mut gc_cnt = 0;
352+
let mut gc_count = 0;
353353
loop {
354-
gc_cnt += 1;
355-
if gc_cnt > 1000 {
354+
gc_count += 1;
355+
if gc_count > 1000 {
356356
#[cfg(feature = "gc_bacon")]
357357
{
358358
crate::object::try_gc();
359359
}
360-
gc_cnt = 0;
360+
gc_count = 0;
361361
}
362362
let idx = self.lasti() as usize;
363363
self.update_lasti(|i| *i += 1);

‎vm/src/object/core.rs‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,22 +86,19 @@ pub static ID2TYPE: Lazy<std::sync::Mutex<std::collections::HashMap<TypeId, Stri
8686
/// A type to just represent "we've erased the type of this object, cast it before you use it"
8787
#[derive(Debug)]
8888
struct Erased;
89-
cfg_if::cfg_if! {
90-
if #[cfg(feature = "gc_bacon")] {
91-
struct PyObjVTable {
92-
drop_dealloc: unsafe fn(*mut PyObject),
93-
drop_only: unsafe fn(*mut PyObject),
94-
dealloc_only: unsafe fn(*mut PyObject),
95-
debug: unsafe fn(&PyObject, &mut fmt::Formatter) -> fmt::Result,
96-
trace: Option<unsafe fn(&PyObject, &mut TracerFn)>
97-
}
98-
}
99-
else{
100-
struct PyObjVTable {
101-
drop_dealloc: unsafe fn(*mut PyObject),
102-
debug: unsafe fn(&PyObject, &mut fmt::Formatter) -> fmt::Result,
103-
}
104-
}
89+
#[cfg(feature = "gc_bacon")]
90+
struct PyObjVTable {
91+
drop_dealloc: unsafe fn(*mut PyObject),
92+
drop_only: unsafe fn(*mut PyObject),
93+
dealloc_only: unsafe fn(*mut PyObject),
94+
debug: unsafe fn(&PyObject, &mut fmt::Formatter) -> fmt::Result,
95+
trace: Option<unsafe fn(&PyObject, &mut TracerFn)>,
96+
}
97+
98+
#[cfg(not(feature = "gc_bacon"))]
99+
struct PyObjVTable {
100+
drop_dealloc: unsafe fn(*mut PyObject),
101+
debug: unsafe fn(&PyObject, &mut fmt::Formatter) -> fmt::Result,
105102
}
106103

107104
unsafe fn drop_dealloc_obj<T: PyObjectPayload>(x: *mut PyObject) {
@@ -1087,14 +1084,10 @@ impl PyObject {
10871084
#[cfg(feature = "gc_bacon")]
10881085
/// only run `__del__`(or `slot_del` depends on the actual object), doesn't drop or dealloc
10891086
pub(in crate::object) unsafe fn del_only(ptr: NonNull<Self>) -> bool {
1090-
if let Err(()) = ptr.as_ref().try_del() {
1091-
return false;
1092-
}
1093-
true
1087+
ptr.as_ref().try_del().is_ok()
10941088
}
10951089

10961090
/// Can only be called when ref_count has dropped to zero. `ptr` must be valid, it run __del__ then drop&dealloc
1097-
#[inline(never)]
10981091
pub(in crate::object) unsafe fn drop_slow(ptr: NonNull<PyObject>) -> bool {
10991092
if let Err(()) = ptr.as_ref().drop_slow_inner() {
11001093
// abort drop for whatever reason

‎vm/src/object/gc/collector.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ impl Collector {
252252
});
253253
}
254254

255+
/// TODO: change to use weak_ref count to prevent premature dealloc in cycles
255256
/// free everything in white, safe to use even when those objects form cycle refs
256257
fn free_cycles(&self, white: Vec<NonNull<PyObject>>) -> usize {
257258
// TODO: maybe never run __del__ anyway, for running a __del__ function is an implementation detail!!!!

‎vm/src/object/payload.rs‎

Lines changed: 114 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -5,135 +5,137 @@ use crate::{
55
vm::VirtualMachine,
66
};
77

8-
cfg_if::cfg_if! {
9-
if #[cfg(feature = "threading")] {
10-
pub trait PyThreadingConstraint: Send + Sync {}
11-
impl<T: Send + Sync> PyThreadingConstraint for T {}
12-
} else {
13-
pub trait PyThreadingConstraint {}
14-
impl<T> PyThreadingConstraint for T {}
15-
}
16-
}
8+
#[cfg(feature = "threading")]
9+
pub trait PyThreadingConstraint: Send + Sync {}
10+
#[cfg(feature = "threading")]
11+
impl<T: Send + Sync> PyThreadingConstraint for T {}
12+
#[cfg(not(feature = "threading"))]
13+
pub trait PyThreadingConstraint {}
14+
#[cfg(not(feature = "threading"))]
15+
impl<T> PyThreadingConstraint for T {}
1716

18-
cfg_if::cfg_if! {
19-
if #[cfg(feature = "gc_bacon")] {
20-
use crate::object::MaybeTrace;
21-
pub trait PyPayload: std::fmt::Debug + PyThreadingConstraint + Sized + MaybeTrace + 'static {
22-
fn class(vm: &VirtualMachine) -> &'static Py<PyType>;
17+
#[cfg(feature = "gc_bacon")]
18+
use crate::object::MaybeTrace;
19+
#[cfg(feature = "gc_bacon")]
20+
pub trait PyPayload:
21+
std::fmt::Debug + PyThreadingConstraint + Sized + MaybeTrace + 'static
22+
{
23+
fn class(vm: &VirtualMachine) -> &'static Py<PyType>;
2324

24-
#[inline]
25-
fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef {
26-
self.into_ref(vm).into()
27-
}
25+
#[inline]
26+
fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef {
27+
self.into_ref(vm).into()
28+
}
2829

29-
#[inline(always)]
30-
fn special_retrieve(_vm: &VirtualMachine, _obj: &PyObject) -> Option<PyResult<PyRef<Self>>> {
31-
None
32-
}
30+
#[inline(always)]
31+
fn special_retrieve(_vm: &VirtualMachine, _obj: &PyObject) -> Option<PyResult<PyRef<Self>>> {
32+
None
33+
}
3334

34-
#[inline]
35-
fn _into_ref(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyRef<Self> {
36-
let dict = if cls.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
37-
Some(vm.ctx.new_dict())
38-
} else {
39-
None
40-
};
41-
PyRef::new_ref(self, cls, dict)
42-
}
35+
#[inline]
36+
fn _into_ref(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyRef<Self> {
37+
let dict = if cls.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
38+
Some(vm.ctx.new_dict())
39+
} else {
40+
None
41+
};
42+
PyRef::new_ref(self, cls, dict)
43+
}
4344

44-
#[inline]
45-
fn into_ref(self, vm: &VirtualMachine) -> PyRef<Self> {
46-
let cls = Self::class(vm);
47-
self._into_ref(cls.to_owned(), vm)
48-
}
45+
#[inline]
46+
fn into_ref(self, vm: &VirtualMachine) -> PyRef<Self> {
47+
let cls = Self::class(vm);
48+
self._into_ref(cls.to_owned(), vm)
49+
}
4950

50-
#[inline]
51-
fn into_ref_with_type(self, vm: &VirtualMachine, cls: PyTypeRef) -> PyResult<PyRef<Self>> {
52-
let exact_class = Self::class(vm);
53-
if cls.fast_issubclass(exact_class) {
54-
Ok(self._into_ref(cls, vm))
55-
} else {
56-
#[cold]
57-
#[inline(never)]
58-
fn _into_ref_with_type_error(
59-
vm: &VirtualMachine,
60-
cls: &PyTypeRef,
61-
exact_class: &Py<PyType>,
62-
) -> PyBaseExceptionRef {
63-
vm.new_type_error(format!(
64-
"'{}' is not a subtype of '{}'",
65-
&cls.name(),
66-
exact_class.name()
67-
))
68-
}
69-
Err(_into_ref_with_type_error(vm, &cls, exact_class))
70-
}
51+
#[inline]
52+
fn into_ref_with_type(self, vm: &VirtualMachine, cls: PyTypeRef) -> PyResult<PyRef<Self>> {
53+
let exact_class = Self::class(vm);
54+
if cls.fast_issubclass(exact_class) {
55+
Ok(self._into_ref(cls, vm))
56+
} else {
57+
#[cold]
58+
#[inline(never)]
59+
fn _into_ref_with_type_error(
60+
vm: &VirtualMachine,
61+
cls: &PyTypeRef,
62+
exact_class: &Py<PyType>,
63+
) -> PyBaseExceptionRef {
64+
vm.new_type_error(format!(
65+
"'{}' is not a subtype of '{}'",
66+
&cls.name(),
67+
exact_class.name()
68+
))
7169
}
70+
Err(_into_ref_with_type_error(vm, &cls, exact_class))
7271
}
72+
}
73+
}
7374

74-
pub trait PyObjectPayload:
75-
std::any::Any + std::fmt::Debug + PyThreadingConstraint + MaybeTrace + 'static
76-
{
77-
}
78-
}else{
79-
pub trait PyPayload: std::fmt::Debug + PyThreadingConstraint + Sized + 'static {
80-
fn class(vm: &VirtualMachine) -> &'static Py<PyType>;
75+
#[cfg(feature = "gc_bacon")]
76+
pub trait PyObjectPayload:
77+
std::any::Any + std::fmt::Debug + PyThreadingConstraint + MaybeTrace + 'static
78+
{
79+
}
8180

82-
#[inline]
83-
fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef {
84-
self.into_ref(vm).into()
85-
}
81+
#[cfg(not(feature = "gc_bacon"))]
82+
pub trait PyPayload: std::fmt::Debug + PyThreadingConstraint + Sized + 'static {
83+
fn class(vm: &VirtualMachine) -> &'static Py<PyType>;
8684

87-
#[inline(always)]
88-
fn special_retrieve(_vm: &VirtualMachine, _obj: &PyObject) -> Option<PyResult<PyRef<Self>>> {
89-
None
90-
}
85+
#[inline]
86+
fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef {
87+
self.into_ref(vm).into()
88+
}
9189

92-
#[inline]
93-
fn _into_ref(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyRef<Self> {
94-
let dict = if cls.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
95-
Some(vm.ctx.new_dict())
96-
} else {
97-
None
98-
};
99-
PyRef::new_ref(self, cls, dict)
100-
}
90+
#[inline(always)]
91+
fn special_retrieve(_vm: &VirtualMachine, _obj: &PyObject) -> Option<PyResult<PyRef<Self>>> {
92+
None
93+
}
10194

102-
#[inline]
103-
fn into_ref(self, vm: &VirtualMachine) -> PyRef<Self> {
104-
let cls = Self::class(vm);
105-
self._into_ref(cls.to_owned(), vm)
106-
}
95+
#[inline]
96+
fn _into_ref(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyRef<Self> {
97+
let dict = if cls.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
98+
Some(vm.ctx.new_dict())
99+
} else {
100+
None
101+
};
102+
PyRef::new_ref(self, cls, dict)
103+
}
107104

108-
#[inline]
109-
fn into_ref_with_type(self, vm: &VirtualMachine, cls: PyTypeRef) -> PyResult<PyRef<Self>> {
110-
let exact_class = Self::class(vm);
111-
if cls.fast_issubclass(exact_class) {
112-
Ok(self._into_ref(cls, vm))
113-
} else {
114-
#[cold]
115-
#[inline(never)]
116-
fn _into_ref_with_type_error(
117-
vm: &VirtualMachine,
118-
cls: &PyTypeRef,
119-
exact_class: &Py<PyType>,
120-
) -> PyBaseExceptionRef {
121-
vm.new_type_error(format!(
122-
"'{}' is not a subtype of '{}'",
123-
&cls.name(),
124-
exact_class.name()
125-
))
126-
}
127-
Err(_into_ref_with_type_error(vm, &cls, exact_class))
128-
}
129-
}
130-
}
105+
#[inline]
106+
fn into_ref(self, vm: &VirtualMachine) -> PyRef<Self> {
107+
let cls = Self::class(vm);
108+
self._into_ref(cls.to_owned(), vm)
109+
}
131110

132-
pub trait PyObjectPayload:
133-
std::any::Any + std::fmt::Debug + PyThreadingConstraint + 'static
134-
{
111+
#[inline]
112+
fn into_ref_with_type(self, vm: &VirtualMachine, cls: PyTypeRef) -> PyResult<PyRef<Self>> {
113+
let exact_class = Self::class(vm);
114+
if cls.fast_issubclass(exact_class) {
115+
Ok(self._into_ref(cls, vm))
116+
} else {
117+
#[cold]
118+
#[inline(never)]
119+
fn _into_ref_with_type_error(
120+
vm: &VirtualMachine,
121+
cls: &PyTypeRef,
122+
exact_class: &Py<PyType>,
123+
) -> PyBaseExceptionRef {
124+
vm.new_type_error(format!(
125+
"'{}' is not a subtype of '{}'",
126+
&cls.name(),
127+
exact_class.name()
128+
))
129+
}
130+
Err(_into_ref_with_type_error(vm, &cls, exact_class))
135131
}
136132
}
137133
}
138134

135+
#[cfg(not(feature = "gc_bacon"))]
136+
pub trait PyObjectPayload:
137+
std::any::Any + std::fmt::Debug + PyThreadingConstraint + 'static
138+
{
139+
}
140+
139141
impl<T: PyPayload + 'static> PyObjectPayload for T {}

0 commit comments

Comments
 (0)