Skip to content

Commit 5c341ef

Browse files
committed
Implement generator/coroutine lifecycle, tracing, and error handling
- Add interactive REPL mode: auto-print expression results in single mode - Implement Destructor for PyGenerator and PyCoroutine - Add locals_dirty tracking and locals_to_fast() for frame sync - Add per-line tracing with prev_line tracking in execution loop - Fix gen_throw to close sub-iterator on GeneratorExit (gen_close_iter) - Pass callable object as arg in c_call/c_return/c_exception trace events - Distinguish [Errno] vs [WinError] for CRT vs Win32 API errors - Fix tee thread safety with AtomicBool running flag - Fix division error messages to match expected format
1 parent ccd3d4f commit 5c341ef

17 files changed

Lines changed: 337 additions & 68 deletions

File tree

crates/codegen/src/compile.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ struct Compiler {
151151
ctx: CompileContext,
152152
opts: CompileOpts,
153153
in_annotation: bool,
154+
/// True when compiling in "single" (interactive) mode.
155+
/// Expression statements at module scope emit CALL_INTRINSIC_1(Print).
156+
interactive: bool,
154157
}
155158

156159
#[derive(Clone, Copy)]
@@ -461,6 +464,7 @@ impl Compiler {
461464
},
462465
opts,
463466
in_annotation: false,
467+
interactive: false,
464468
}
465469
}
466470

@@ -1706,6 +1710,7 @@ impl Compiler {
17061710
body: &[ast::Stmt],
17071711
symbol_table: SymbolTable,
17081712
) -> CompileResult<()> {
1713+
self.interactive = true;
17091714
// Set future_annotations from symbol table (detected during symbol table scan)
17101715
self.future_annotations = symbol_table.future_annotations;
17111716
self.symbol_table_stack.push(symbol_table);
@@ -2151,7 +2156,15 @@ impl Compiler {
21512156
ast::Stmt::Expr(ast::StmtExpr { value, .. }) => {
21522157
self.compile_expression(value)?;
21532158

2154-
// Pop result of stack, since we not use it:
2159+
if self.interactive && !self.ctx.in_func() && !self.ctx.in_class {
2160+
emit!(
2161+
self,
2162+
Instruction::CallIntrinsic1 {
2163+
func: bytecode::IntrinsicFunction1::Print
2164+
}
2165+
);
2166+
}
2167+
21552168
emit!(self, Instruction::PopTop);
21562169
}
21572170
ast::Stmt::Global(_) | ast::Stmt::Nonlocal(_) => {

crates/vm/src/builtins/coroutine.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
function::OptionalArg,
88
object::{Traverse, TraverseFn},
99
protocol::PyIterReturn,
10-
types::{IterNext, Iterable, Representable, SelfIter},
10+
types::{Destructor, IterNext, Iterable, Representable, SelfIter},
1111
};
1212
use crossbeam_utils::atomic::AtomicCell;
1313

@@ -31,7 +31,10 @@ impl PyPayload for PyCoroutine {
3131
}
3232
}
3333

34-
#[pyclass(flags(DISALLOW_INSTANTIATION), with(Py, IterNext, Representable))]
34+
#[pyclass(
35+
flags(DISALLOW_INSTANTIATION),
36+
with(Py, IterNext, Representable, Destructor)
37+
)]
3538
impl PyCoroutine {
3639
pub const fn as_coro(&self) -> &Coro {
3740
&self.inner
@@ -130,7 +133,7 @@ impl Py<PyCoroutine> {
130133
}
131134

132135
#[pymethod]
133-
fn close(&self, vm: &VirtualMachine) -> PyResult<()> {
136+
fn close(&self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
134137
self.inner.close(self.as_object(), vm)
135138
}
136139
}
@@ -149,6 +152,22 @@ impl IterNext for PyCoroutine {
149152
}
150153
}
151154

155+
impl Destructor for PyCoroutine {
156+
fn del(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<()> {
157+
if zelf.inner.closed() || zelf.inner.running() {
158+
return Ok(());
159+
}
160+
if zelf.inner.frame().lasti() == 0 {
161+
zelf.inner.closed.store(true);
162+
return Ok(());
163+
}
164+
if let Err(e) = zelf.inner.close(zelf.as_object(), vm) {
165+
vm.run_unraisable(e, None, zelf.as_object().to_owned());
166+
}
167+
Ok(())
168+
}
169+
}
170+
152171
#[pyclass(module = false, name = "coroutine_wrapper", traverse = "manual")]
153172
#[derive(Debug)]
154173
// PyCoroWrapper_Type in CPython
@@ -209,7 +228,7 @@ impl PyCoroutineWrapper {
209228
}
210229

211230
#[pymethod]
212-
fn close(&self, vm: &VirtualMachine) -> PyResult<()> {
231+
fn close(&self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
213232
self.closed.store(true);
214233
self.coro.close(vm)
215234
}

crates/vm/src/builtins/frame.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ impl Frame {
4343

4444
#[pygetset]
4545
fn f_locals(&self, vm: &VirtualMachine) -> PyResult {
46-
self.locals(vm).map(Into::into)
46+
let result = self.locals(vm).map(Into::into);
47+
self.locals_dirty
48+
.store(true, core::sync::atomic::Ordering::Release);
49+
result
4750
}
4851

4952
#[pygetset]

crates/vm/src/builtins/generator.rs

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
function::OptionalArg,
1212
object::{Traverse, TraverseFn},
1313
protocol::PyIterReturn,
14-
types::{IterNext, Iterable, Representable, SelfIter},
14+
types::{Destructor, IterNext, Iterable, Representable, SelfIter},
1515
};
1616

1717
#[pyclass(module = false, name = "generator", traverse = "manual")]
@@ -33,7 +33,10 @@ impl PyPayload for PyGenerator {
3333
}
3434
}
3535

36-
#[pyclass(flags(DISALLOW_INSTANTIATION), with(Py, IterNext, Iterable))]
36+
#[pyclass(
37+
flags(DISALLOW_INSTANTIATION),
38+
with(Py, IterNext, Iterable, Representable, Destructor)
39+
)]
3740
impl PyGenerator {
3841
pub const fn as_coro(&self) -> &Coro {
3942
&self.inner
@@ -89,6 +92,11 @@ impl PyGenerator {
8992
self.inner.frame().yield_from_target()
9093
}
9194

95+
#[pygetset]
96+
fn gi_suspended(&self, _vm: &VirtualMachine) -> bool {
97+
self.inner.suspended()
98+
}
99+
92100
#[pyclassmethod]
93101
fn __class_getitem__(cls: PyTypeRef, args: PyObjectRef, vm: &VirtualMachine) -> PyGenericAlias {
94102
PyGenericAlias::from_args(cls, args, vm)
@@ -121,7 +129,7 @@ impl Py<PyGenerator> {
121129
}
122130

123131
#[pymethod]
124-
fn close(&self, vm: &VirtualMachine) -> PyResult<()> {
132+
fn close(&self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
125133
self.inner.close(self.as_object(), vm)
126134
}
127135
}
@@ -140,6 +148,25 @@ impl IterNext for PyGenerator {
140148
}
141149
}
142150

151+
impl Destructor for PyGenerator {
152+
fn del(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<()> {
153+
// _PyGen_Finalize: close the generator if it's still suspended
154+
if zelf.inner.closed() || zelf.inner.running() {
155+
return Ok(());
156+
}
157+
// Generator was never started, just mark as closed
158+
if zelf.inner.frame().lasti() == 0 {
159+
zelf.inner.closed.store(true);
160+
return Ok(());
161+
}
162+
// Throw GeneratorExit to run finally blocks
163+
if let Err(e) = zelf.inner.close(zelf.as_object(), vm) {
164+
vm.run_unraisable(e, None, zelf.as_object().to_owned());
165+
}
166+
Ok(())
167+
}
168+
}
169+
143170
impl Drop for PyGenerator {
144171
fn drop(&mut self) {
145172
self.inner.frame().clear_generator();

crates/vm/src/builtins/int.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,23 +122,23 @@ fn inner_pow(int1: &BigInt, int2: &BigInt, vm: &VirtualMachine) -> PyResult {
122122

123123
fn inner_mod(int1: &BigInt, int2: &BigInt, vm: &VirtualMachine) -> PyResult {
124124
if int2.is_zero() {
125-
Err(vm.new_zero_division_error("integer modulo by zero"))
125+
Err(vm.new_zero_division_error("division by zero"))
126126
} else {
127127
Ok(vm.ctx.new_int(int1.mod_floor(int2)).into())
128128
}
129129
}
130130

131131
fn inner_floordiv(int1: &BigInt, int2: &BigInt, vm: &VirtualMachine) -> PyResult {
132132
if int2.is_zero() {
133-
Err(vm.new_zero_division_error("integer division or modulo by zero"))
133+
Err(vm.new_zero_division_error("division by zero"))
134134
} else {
135135
Ok(vm.ctx.new_int(int1.div_floor(int2)).into())
136136
}
137137
}
138138

139139
fn inner_divmod(int1: &BigInt, int2: &BigInt, vm: &VirtualMachine) -> PyResult {
140140
if int2.is_zero() {
141-
return Err(vm.new_zero_division_error("integer division or modulo by zero"));
141+
return Err(vm.new_zero_division_error("division by zero"));
142142
}
143143
let (div, modulo) = int1.div_mod_floor(int2);
144144
Ok(vm.new_tuple((div, modulo)).into())

crates/vm/src/builtins/list.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,13 @@ impl PyList {
225225

226226
fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult {
227227
match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? {
228-
SequenceIndex::Int(i) => self.borrow_vec().getitem_by_index(vm, i),
228+
SequenceIndex::Int(i) => {
229+
let vec = self.borrow_vec();
230+
let pos = vec
231+
.wrap_index(i)
232+
.ok_or_else(|| vm.new_index_error("list index out of range"))?;
233+
Ok(vec.do_get(pos))
234+
}
229235
SequenceIndex::Slice(slice) => self
230236
.borrow_vec()
231237
.getitem_by_slice(vm, slice)
@@ -448,9 +454,12 @@ impl AsSequence for PyList {
448454
.map(|x| x.into())
449455
}),
450456
item: atomic_func!(|seq, i, vm| {
451-
PyList::sequence_downcast(seq)
452-
.borrow_vec()
453-
.getitem_by_index(vm, i)
457+
let list = PyList::sequence_downcast(seq);
458+
let vec = list.borrow_vec();
459+
let pos = vec
460+
.wrap_index(i)
461+
.ok_or_else(|| vm.new_index_error("list index out of range"))?;
462+
Ok(vec.do_get(pos))
454463
}),
455464
ass_item: atomic_func!(|seq, i, value, vm| {
456465
let zelf = PyList::sequence_downcast(seq);

crates/vm/src/coroutine.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{
2-
AsObject, Py, PyObject, PyObjectRef, PyResult, VirtualMachine,
2+
AsObject, Py, PyObject, PyObjectRef, PyResult, TryFromObject, VirtualMachine,
33
builtins::{PyBaseExceptionRef, PyStrRef},
44
common::lock::PyMutex,
55
exceptions::types::PyBaseException,
@@ -135,6 +135,7 @@ impl Coro {
135135
if self.closed.load() {
136136
return Ok(PyIterReturn::StopIteration(None));
137137
}
138+
self.frame.locals_to_fast(vm)?;
138139
let value = if self.frame.lasti() > 0 {
139140
Some(value)
140141
} else if !vm.is_none(&value) {
@@ -176,22 +177,37 @@ impl Coro {
176177
exc_tb: PyObjectRef,
177178
vm: &VirtualMachine,
178179
) -> PyResult<PyIterReturn> {
180+
// Validate throw arguments (matching CPython _gen_throw)
181+
if exc_type.fast_isinstance(vm.ctx.exceptions.base_exception_type) && !vm.is_none(&exc_val)
182+
{
183+
return Err(
184+
vm.new_type_error("instance exception may not have a separate value".to_owned())
185+
);
186+
}
187+
if !vm.is_none(&exc_tb) && !exc_tb.fast_isinstance(vm.ctx.types.traceback_type) {
188+
return Err(
189+
vm.new_type_error("throw() third argument must be a traceback object".to_owned())
190+
);
191+
}
179192
if self.closed.load() {
180193
return Err(vm.normalize_exception(exc_type, exc_val, exc_tb)?);
181194
}
195+
// Validate exception type before entering generator context.
196+
// Invalid types propagate to caller without closing the generator.
197+
crate::exceptions::ExceptionCtor::try_from_object(vm, exc_type.clone())?;
182198
let result = self.run_with_context(jen, vm, |f| f.gen_throw(vm, exc_type, exc_val, exc_tb));
183199
self.maybe_close(&result);
184200
Ok(result?.into_iter_return(vm))
185201
}
186202

187-
pub fn close(&self, jen: &PyObject, vm: &VirtualMachine) -> PyResult<()> {
203+
pub fn close(&self, jen: &PyObject, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
188204
if self.closed.load() {
189-
return Ok(());
205+
return Ok(vm.ctx.none());
190206
}
191207
// If generator hasn't started (FRAME_CREATED), just mark as closed
192208
if self.frame.lasti() == 0 {
193209
self.closed.store(true);
194-
return Ok(());
210+
return Ok(vm.ctx.none());
195211
}
196212
let result = self.run_with_context(jen, vm, |f| {
197213
f.gen_throw(
@@ -207,10 +223,15 @@ impl Coro {
207223
Err(vm.new_runtime_error(format!("{} ignored GeneratorExit", gen_name(jen, vm))))
208224
}
209225
Err(e) if !is_gen_exit(&e, vm) => Err(e),
210-
_ => Ok(()),
226+
Ok(ExecutionResult::Return(value)) => Ok(value),
227+
_ => Ok(vm.ctx.none()),
211228
}
212229
}
213230

231+
pub fn suspended(&self) -> bool {
232+
!self.closed.load() && !self.running.load() && self.frame.lasti() > 0
233+
}
234+
214235
pub fn running(&self) -> bool {
215236
self.running.load()
216237
}
@@ -240,10 +261,11 @@ impl Coro {
240261
}
241262

242263
pub fn repr(&self, jen: &PyObject, id: usize, vm: &VirtualMachine) -> String {
264+
let qualname = self.qualname();
243265
format!(
244266
"<{} object {} at {:#x}>",
245267
gen_name(jen, vm),
246-
self.name.lock(),
268+
qualname.as_str(),
247269
id
248270
)
249271
}

crates/vm/src/exceptions.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,13 @@ impl VirtualMachine {
348348
) -> PyResult<PyBaseExceptionRef> {
349349
// TODO: fast-path built-in exceptions by directly instantiating them? Is that really worth it?
350350
let res = PyType::call(&cls, args.into_args(self), self)?;
351-
PyBaseExceptionRef::try_from_object(self, res)
351+
res.downcast::<PyBaseException>().map_err(|obj| {
352+
self.new_type_error(format!(
353+
"calling {} should have returned an instance of BaseException, not {}",
354+
cls,
355+
obj.class()
356+
))
357+
})
352358
}
353359
}
354360

@@ -1307,6 +1313,16 @@ impl OSErrorBuilder {
13071313
self
13081314
}
13091315

1316+
/// Strip winerror from the builder. Used for C runtime errors
1317+
/// (e.g. `_wopen`, `open`) that should produce `[Errno X]` format
1318+
/// instead of `[WinError X]`.
1319+
#[must_use]
1320+
#[cfg(windows)]
1321+
pub(crate) fn without_winerror(mut self) -> Self {
1322+
self.winerror = None;
1323+
self
1324+
}
1325+
13101326
pub fn build(self, vm: &VirtualMachine) -> PyRef<types::PyOSError> {
13111327
use types::PyOSError;
13121328

@@ -1391,12 +1407,10 @@ impl ToOSErrorBuilder for std::io::Error {
13911407

13921408
#[allow(unused_mut)]
13931409
let mut builder = OSErrorBuilder::with_errno(errno, msg, vm);
1394-
13951410
#[cfg(windows)]
13961411
if let Some(winerror) = self.raw_os_error() {
13971412
builder = builder.winerror(winerror.to_pyobject(vm));
13981413
}
1399-
14001414
builder
14011415
}
14021416
}

0 commit comments

Comments
 (0)