Fix trace_event to return trace function result

- Return trace function's return value from trace_event()
  to support per-frame f_trace assignment
- Match CPython's trace_trampoline: set f_trace from call
  event return value, clear on error
- Fire return event only when frame is traced or profiled
- Remove expectedFailure from passing bdb/settrace tests
This commit is contained in:
Jeong, YunWon
2026-02-28 10:32:15 +09:00
parent 78c5a2ec28
commit 754fc85fb8
4 changed files with 65 additions and 38 deletions

View File

@@ -762,7 +762,6 @@ class StateTestCase(BaseTestCase):
bdb = Bdb(skip=['anything*'])
self.assertIs(bdb.is_skipped_module(None), False)
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
def test_down(self):
# Check that set_down() raises BdbError at the newest frame.
self.expect_set = [
@@ -784,7 +783,6 @@ class StateTestCase(BaseTestCase):
class BreakpointTestCase(BaseTestCase):
"""Test the breakpoint set method."""
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
def test_bp_on_non_existent_module(self):
self.expect_set = [
('line', 2, 'tfunc_import'), ('break', ('/non/existent/module.py', 1))
@@ -792,7 +790,6 @@ class BreakpointTestCase(BaseTestCase):
with TracerRun(self) as tracer:
self.assertRaises(BdbError, tracer.runcall, tfunc_import)
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
def test_bp_after_last_statement(self):
code = """
def main():
@@ -969,7 +966,6 @@ class BreakpointTestCase(BaseTestCase):
with TracerRun(self) as tracer:
tracer.runcall(tfunc_import)
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
def test_clear_at_no_bp(self):
self.expect_set = [
('line', 2, 'tfunc_import'), ('clear', (__file__, 1))

View File

@@ -1218,8 +1218,6 @@ class RaisingTraceFuncTestCase(unittest.TestCase):
def test_exception(self):
self.run_test_for_event('exception')
# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_trash_stack(self):
def f():
for i in range(5):
@@ -1785,15 +1783,11 @@ class JumpTestCase(unittest.TestCase):
# The second set of 'jump' tests are for things that are not allowed:
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(2, 3, [1], (ValueError, 'after'))
def test_no_jump_too_far_forwards(output):
output.append(1)
output.append(2)
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(2, -2, [1], (ValueError, 'before'))
def test_no_jump_too_far_backwards(output):
output.append(1)
@@ -1840,8 +1834,6 @@ class JumpTestCase(unittest.TestCase):
output.append(4)
raise e
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(1, 3, [], (ValueError, 'into'))
def test_no_jump_forwards_into_for_block(output):
output.append(1)
@@ -1857,8 +1849,6 @@ class JumpTestCase(unittest.TestCase):
output.append(3)
pass
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(3, 2, [2, 2], (ValueError, 'into'))
def test_no_jump_backwards_into_for_block(output):
for i in 1, 2:
@@ -2020,8 +2010,6 @@ class JumpTestCase(unittest.TestCase):
raise
output.append(8)
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(3, 6, [2], (ValueError, "into an 'except'"))
def test_no_jump_into_qualified_except_block_from_try_block(output):
try:
@@ -2087,8 +2075,6 @@ class JumpTestCase(unittest.TestCase):
return
output.append(7)
# TODO: RUSTPYTHON
@unittest.expectedFailure
@jump_test(7, 4, [1, 6], (ValueError, 'into'))
def test_no_jump_into_for_block_before_else(output):
output.append(1)

View File

@@ -96,37 +96,67 @@ impl core::fmt::Display for TraceEvent {
impl VirtualMachine {
/// Call registered trace function.
///
/// Returns the trace function's return value:
/// - `Some(obj)` if the trace function returned a non-None value
/// - `None` if it returned Python None or no trace function was active
///
/// In CPython's trace protocol:
/// - For 'call' events: the return value determines the per-frame `f_trace`
/// - For 'line'/'return' events: the return value can update `f_trace`
#[inline]
pub(crate) fn trace_event(&self, event: TraceEvent, arg: Option<PyObjectRef>) -> PyResult<()> {
pub(crate) fn trace_event(
&self,
event: TraceEvent,
arg: Option<PyObjectRef>,
) -> PyResult<Option<PyObjectRef>> {
if self.use_tracing.get() {
self._trace_event_inner(event, arg)
} else {
Ok(())
Ok(None)
}
}
fn _trace_event_inner(&self, event: TraceEvent, arg: Option<PyObjectRef>) -> PyResult<()> {
fn _trace_event_inner(
&self,
event: TraceEvent,
arg: Option<PyObjectRef>,
) -> PyResult<Option<PyObjectRef>> {
let trace_func = self.trace_func.borrow().to_owned();
let profile_func = self.profile_func.borrow().to_owned();
if self.is_none(&trace_func) && self.is_none(&profile_func) {
return Ok(());
return Ok(None);
}
let Some(frame_ref) = self.current_frame() else {
return Ok(());
return Ok(None);
};
let frame: PyObjectRef = frame_ref.into();
let event = self.ctx.new_str(event.to_string()).into();
let args = vec![frame, event, arg.unwrap_or_else(|| self.ctx.none())];
let mut trace_result = None;
// temporarily disable tracing, during the call to the
// tracing function itself.
if !self.is_none(&trace_func) {
self.use_tracing.set(false);
let res = trace_func.call(args.clone(), self);
self.use_tracing.set(true);
if res.is_err() {
*self.trace_func.borrow_mut() = self.ctx.none();
match res {
Ok(result) => {
if !self.is_none(&result) {
trace_result = Some(result);
}
}
Err(e) => {
// trace_trampoline behavior: clear per-frame f_trace
// and propagate the error.
if let Some(frame_ref) = self.current_frame() {
*frame_ref.trace.lock() = self.ctx.none();
}
return Err(e);
}
}
}
@@ -138,6 +168,6 @@ impl VirtualMachine {
*self.profile_func.borrow_mut() = self.ctx.none();
}
}
Ok(())
Ok(trace_result)
}
}

View File

@@ -1084,19 +1084,25 @@ impl VirtualMachine {
// Fire 'call' trace event after pushing frame
// (current_frame() now returns the callee's frame)
//
// trace_dispatch protocol (matching CPython's trace_trampoline):
// - For 'call' events, the global trace function is called.
// If it returns non-None, set f_trace to that value (trace this frame).
// If it returns None, leave f_trace unset (skip tracing this frame).
// - For 'return' events, fire if this frame has f_trace set OR if
// a profile function is active (profiling is independent of f_trace).
match self.trace_event(TraceEvent::Call, None) {
Ok(()) => {
// Set per-frame trace function so line events fire for this frame.
// Frames entered before sys.settrace() keep trace=None and skip line events.
if self.use_tracing.get() {
let trace_func = self.trace_func.borrow().clone();
if !self.is_none(&trace_func) {
*frame.trace.lock() = trace_func;
}
Ok(trace_result) => {
if let Some(local_trace) = trace_result {
*frame.trace.lock() = local_trace;
}
let result = f(frame.clone());
// Fire 'return' trace event on success
if result.is_ok() {
// Fire 'return' event if frame is being traced or profiled
if result.is_ok()
&& self.use_tracing.get()
&& (!self.is_none(&frame.trace.lock())
|| !self.is_none(&self.profile_func.borrow()))
{
let _ = self.trace_event(TraceEvent::Return, None);
}
result
@@ -1155,9 +1161,18 @@ impl VirtualMachine {
use crate::protocol::TraceEvent;
match self.trace_event(TraceEvent::Call, None) {
Ok(()) => {
Ok(trace_result) => {
// Update per-frame trace if trace function returned a new local trace
if let Some(local_trace) = trace_result {
*frame.trace.lock() = local_trace;
}
let result = f(frame);
if result.is_ok() {
// Fire 'return' event if frame is being traced or profiled
if result.is_ok()
&& self.use_tracing.get()
&& (!self.is_none(&frame.trace.lock())
|| !self.is_none(&self.profile_func.borrow()))
{
let _ = self.trace_event(TraceEvent::Return, None);
}
result