Skip to content

Commit 754fc85

Browse files
youknowoneyouknowone
committed
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
1 parent 78c5a2e commit 754fc85

File tree

4 files changed

+65
-38
lines changed

4 files changed

+65
-38
lines changed

‎Lib/test/test_bdb.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,6 @@ def test_skip_with_no_name_module(self):
762762
bdb = Bdb(skip=['anything*'])
763763
self.assertIs(bdb.is_skipped_module(None), False)
764764

765-
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
766765
def test_down(self):
767766
# Check that set_down() raises BdbError at the newest frame.
768767
self.expect_set = [
@@ -784,15 +783,13 @@ def test_up(self):
784783
class BreakpointTestCase(BaseTestCase):
785784
"""Test the breakpoint set method."""
786785

787-
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
788786
def test_bp_on_non_existent_module(self):
789787
self.expect_set = [
790788
('line', 2, 'tfunc_import'), ('break', ('/non/existent/module.py', 1))
791789
]
792790
with TracerRun(self) as tracer:
793791
self.assertRaises(BdbError, tracer.runcall, tfunc_import)
794792

795-
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
796793
def test_bp_after_last_statement(self):
797794
code = """
798795
def main():
@@ -969,7 +966,6 @@ def main():
969966
with TracerRun(self) as tracer:
970967
tracer.runcall(tfunc_import)
971968

972-
@unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs
973969
def test_clear_at_no_bp(self):
974970
self.expect_set = [
975971
('line', 2, 'tfunc_import'), ('clear', (__file__, 1))

‎Lib/test/test_sys_settrace.py‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,8 +1218,6 @@ def test_return(self):
12181218
def test_exception(self):
12191219
self.run_test_for_event('exception')
12201220

1221-
# TODO: RUSTPYTHON
1222-
@unittest.expectedFailure
12231221
def test_trash_stack(self):
12241222
def f():
12251223
for i in range(5):
@@ -1785,15 +1783,11 @@ async def test_jump_over_async_for_block_before_else(output):
17851783

17861784
# The second set of 'jump' tests are for things that are not allowed:
17871785

1788-
# TODO: RUSTPYTHON
1789-
@unittest.expectedFailure
17901786
@jump_test(2, 3, [1], (ValueError, 'after'))
17911787
def test_no_jump_too_far_forwards(output):
17921788
output.append(1)
17931789
output.append(2)
17941790

1795-
# TODO: RUSTPYTHON
1796-
@unittest.expectedFailure
17971791
@jump_test(2, -2, [1], (ValueError, 'before'))
17981792
def test_no_jump_too_far_backwards(output):
17991793
output.append(1)
@@ -1840,8 +1834,6 @@ def test_no_jump_to_except_4(output):
18401834
output.append(4)
18411835
raise e
18421836

1843-
# TODO: RUSTPYTHON
1844-
@unittest.expectedFailure
18451837
@jump_test(1, 3, [], (ValueError, 'into'))
18461838
def test_no_jump_forwards_into_for_block(output):
18471839
output.append(1)
@@ -1857,8 +1849,6 @@ async def test_no_jump_forwards_into_async_for_block(output):
18571849
output.append(3)
18581850
pass
18591851

1860-
# TODO: RUSTPYTHON
1861-
@unittest.expectedFailure
18621852
@jump_test(3, 2, [2, 2], (ValueError, 'into'))
18631853
def test_no_jump_backwards_into_for_block(output):
18641854
for i in 1, 2:
@@ -2020,8 +2010,6 @@ def test_no_jump_into_bare_except_block_from_try_block(output):
20202010
raise
20212011
output.append(8)
20222012

2023-
# TODO: RUSTPYTHON
2024-
@unittest.expectedFailure
20252013
@jump_test(3, 6, [2], (ValueError, "into an 'except'"))
20262014
def test_no_jump_into_qualified_except_block_from_try_block(output):
20272015
try:
@@ -2087,8 +2075,6 @@ def test_no_jump_over_return_out_of_finally_block(output):
20872075
return
20882076
output.append(7)
20892077

2090-
# TODO: RUSTPYTHON
2091-
@unittest.expectedFailure
20922078
@jump_test(7, 4, [1, 6], (ValueError, 'into'))
20932079
def test_no_jump_into_for_block_before_else(output):
20942080
output.append(1)

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

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,37 +96,67 @@ impl core::fmt::Display for TraceEvent {
9696

9797
impl VirtualMachine {
9898
/// Call registered trace function.
99+
///
100+
/// Returns the trace function's return value:
101+
/// - `Some(obj)` if the trace function returned a non-None value
102+
/// - `None` if it returned Python None or no trace function was active
103+
///
104+
/// In CPython's trace protocol:
105+
/// - For 'call' events: the return value determines the per-frame `f_trace`
106+
/// - For 'line'/'return' events: the return value can update `f_trace`
99107
#[inline]
100-
pub(crate) fn trace_event(&self, event: TraceEvent, arg: Option<PyObjectRef>) -> PyResult<()> {
108+
pub(crate) fn trace_event(
109+
&self,
110+
event: TraceEvent,
111+
arg: Option<PyObjectRef>,
112+
) -> PyResult<Option<PyObjectRef>> {
101113
if self.use_tracing.get() {
102114
self._trace_event_inner(event, arg)
103115
} else {
104-
Ok(())
116+
Ok(None)
105117
}
106118
}
107-
fn _trace_event_inner(&self, event: TraceEvent, arg: Option<PyObjectRef>) -> PyResult<()> {
119+
fn _trace_event_inner(
120+
&self,
121+
event: TraceEvent,
122+
arg: Option<PyObjectRef>,
123+
) -> PyResult<Option<PyObjectRef>> {
108124
let trace_func = self.trace_func.borrow().to_owned();
109125
let profile_func = self.profile_func.borrow().to_owned();
110126
if self.is_none(&trace_func) && self.is_none(&profile_func) {
111-
return Ok(());
127+
return Ok(None);
112128
}
113129

114130
let Some(frame_ref) = self.current_frame() else {
115-
return Ok(());
131+
return Ok(None);
116132
};
117133

118134
let frame: PyObjectRef = frame_ref.into();
119135
let event = self.ctx.new_str(event.to_string()).into();
120136
let args = vec![frame, event, arg.unwrap_or_else(|| self.ctx.none())];
121137

138+
let mut trace_result = None;
139+
122140
// temporarily disable tracing, during the call to the
123141
// tracing function itself.
124142
if !self.is_none(&trace_func) {
125143
self.use_tracing.set(false);
126144
let res = trace_func.call(args.clone(), self);
127145
self.use_tracing.set(true);
128-
if res.is_err() {
129-
*self.trace_func.borrow_mut() = self.ctx.none();
146+
match res {
147+
Ok(result) => {
148+
if !self.is_none(&result) {
149+
trace_result = Some(result);
150+
}
151+
}
152+
Err(e) => {
153+
// trace_trampoline behavior: clear per-frame f_trace
154+
// and propagate the error.
155+
if let Some(frame_ref) = self.current_frame() {
156+
*frame_ref.trace.lock() = self.ctx.none();
157+
}
158+
return Err(e);
159+
}
130160
}
131161
}
132162

@@ -138,6 +168,6 @@ impl VirtualMachine {
138168
*self.profile_func.borrow_mut() = self.ctx.none();
139169
}
140170
}
141-
Ok(())
171+
Ok(trace_result)
142172
}
143173
}

‎crates/vm/src/vm/mod.rs‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,19 +1084,25 @@ impl VirtualMachine {
10841084

10851085
// Fire 'call' trace event after pushing frame
10861086
// (current_frame() now returns the callee's frame)
1087+
//
1088+
// trace_dispatch protocol (matching CPython's trace_trampoline):
1089+
// - For 'call' events, the global trace function is called.
1090+
// If it returns non-None, set f_trace to that value (trace this frame).
1091+
// If it returns None, leave f_trace unset (skip tracing this frame).
1092+
// - For 'return' events, fire if this frame has f_trace set OR if
1093+
// a profile function is active (profiling is independent of f_trace).
10871094
match self.trace_event(TraceEvent::Call, None) {
1088-
Ok(()) => {
1089-
// Set per-frame trace function so line events fire for this frame.
1090-
// Frames entered before sys.settrace() keep trace=None and skip line events.
1091-
if self.use_tracing.get() {
1092-
let trace_func = self.trace_func.borrow().clone();
1093-
if !self.is_none(&trace_func) {
1094-
*frame.trace.lock() = trace_func;
1095-
}
1095+
Ok(trace_result) => {
1096+
if let Some(local_trace) = trace_result {
1097+
*frame.trace.lock() = local_trace;
10961098
}
10971099
let result = f(frame.clone());
1098-
// Fire 'return' trace event on success
1099-
if result.is_ok() {
1100+
// Fire 'return' event if frame is being traced or profiled
1101+
if result.is_ok()
1102+
&& self.use_tracing.get()
1103+
&& (!self.is_none(&frame.trace.lock())
1104+
|| !self.is_none(&self.profile_func.borrow()))
1105+
{
11001106
let _ = self.trace_event(TraceEvent::Return, None);
11011107
}
11021108
result
@@ -1155,9 +1161,18 @@ impl VirtualMachine {
11551161

11561162
use crate::protocol::TraceEvent;
11571163
match self.trace_event(TraceEvent::Call, None) {
1158-
Ok(()) => {
1164+
Ok(trace_result) => {
1165+
// Update per-frame trace if trace function returned a new local trace
1166+
if let Some(local_trace) = trace_result {
1167+
*frame.trace.lock() = local_trace;
1168+
}
11591169
let result = f(frame);
1160-
if result.is_ok() {
1170+
// Fire 'return' event if frame is being traced or profiled
1171+
if result.is_ok()
1172+
&& self.use_tracing.get()
1173+
&& (!self.is_none(&frame.trace.lock())
1174+
|| !self.is_none(&self.profile_func.borrow()))
1175+
{
11611176
let _ = self.trace_event(TraceEvent::Return, None);
11621177
}
11631178
result

0 commit comments

Comments
 (0)