Address review: interactive print nesting, locals_to_fast flag, trace arg

youknowone · youknowone · commit a8dbe2c2b4af · 2026-02-11T10:13:03.000+09:00
- Skip interactive print for expression statements inside compound
  blocks (if/for/while/with/try) by checking in_conditional_block
- Move locals_dirty flag clear to after sync loop succeeds
- Pass callable object as arg to c_call/c_return/c_exception trace events
diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs
@@ -2156,7 +2156,7 @@ impl Compiler {
             ast::Stmt::Expr(ast::StmtExpr { value, .. }) => {
                 self.compile_expression(value)?;
 
-                if self.interactive && !self.ctx.in_func() && !self.ctx.in_class {
+                if self.interactive && !self.ctx.in_func() && !self.ctx.in_class && self.current_code_info().in_conditional_block == 0 {
                     emit!(
                         self,
                         Instruction::CallIntrinsic1 {
diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs
@@ -264,7 +264,6 @@ impl Frame {
         if !self.locals_dirty.load(atomic::Ordering::Acquire) {
             return Ok(());
         }
-        self.locals_dirty.store(false, atomic::Ordering::Release);
         let code = &**self.code;
         let mut fastlocals = self.fastlocals.lock();
         for (i, &varname) in code.varnames.iter().enumerate() {
@@ -277,6 +276,7 @@ impl Frame {
                 Err(e) => return Err(e),
             }
         }
+        self.locals_dirty.store(false, atomic::Ordering::Release);
         Ok(())
     }
 
@@ -475,7 +475,7 @@ impl ExecutingFrame<'_> {
                 && loc.line.get() != prev_line
             {
                 prev_line = loc.line.get();
-                vm.trace_event(crate::protocol::TraceEvent::Line)?;
+                vm.trace_event(crate::protocol::TraceEvent::Line, None)?;
             }
             self.update_lasti(|i| *i += 1);
             let bytecode::CodeUnit { op, arg } = instructions[idx];
diff --git a/crates/vm/src/protocol/callable.rs b/crates/vm/src/protocol/callable.rs
@@ -2,7 +2,7 @@ use crate::{
     builtins::{PyBoundMethod, PyFunction},
     function::{FuncArgs, IntoFuncArgs},
     types::GenericMethod,
-    {AsObject, PyObject, PyResult, VirtualMachine},
+    {AsObject, PyObject, PyObjectRef, PyResult, VirtualMachine},
 };
 
 impl PyObject {
@@ -57,12 +57,13 @@ impl<'a> PyCallable<'a> {
         if is_python_callable {
             (self.call)(self.obj, args, vm)
         } else {
-            vm.trace_event(TraceEvent::CCall)?;
+            let callable = self.obj.to_owned();
+            vm.trace_event(TraceEvent::CCall, Some(callable.clone()))?;
             let result = (self.call)(self.obj, args, vm);
             if result.is_ok() {
-                vm.trace_event(TraceEvent::CReturn)?;
+                vm.trace_event(TraceEvent::CReturn, Some(callable))?;
             } else {
-                let _ = vm.trace_event(TraceEvent::CException);
+                let _ = vm.trace_event(TraceEvent::CException, Some(callable));
             }
             result
         }
@@ -96,14 +97,18 @@ impl core::fmt::Display for TraceEvent {
 impl VirtualMachine {
     /// Call registered trace function.
     #[inline]
-    pub(crate) fn trace_event(&self, event: TraceEvent) -> PyResult<()> {
+    pub(crate) fn trace_event(
+        &self,
+        event: TraceEvent,
+        arg: Option<PyObjectRef>,
+    ) -> PyResult<()> {
         if self.use_tracing.get() {
-            self._trace_event_inner(event)
+            self._trace_event_inner(event, arg)
         } else {
             Ok(())
         }
     }
-    fn _trace_event_inner(&self, event: TraceEvent) -> PyResult<()> {
+    fn _trace_event_inner(&self, event: TraceEvent, arg: Option<PyObjectRef>) -> PyResult<()> {
         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) {
@@ -117,7 +122,7 @@ impl VirtualMachine {
 
         let frame = frame_ref.unwrap().as_object().to_owned();
         let event = self.ctx.new_str(event.to_string()).into();
-        let args = vec![frame, event, self.ctx.none()];
+        let args = vec![frame, event, arg.unwrap_or_else(|| self.ctx.none())];
 
         // temporarily disable tracing, during the call to the
         // tracing function itself.
diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs
@@ -1021,7 +1021,7 @@ impl VirtualMachine {
             use crate::protocol::TraceEvent;
             // Fire 'call' trace event after pushing frame
             // (current_frame() now returns the callee's frame)
-            let result = match self.trace_event(TraceEvent::Call) {
+            let result = 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.
@@ -1034,7 +1034,7 @@ impl VirtualMachine {
                     let result = f(frame);
                     // Fire 'return' trace event on success
                     if result.is_ok() {
-                        let _ = self.trace_event(TraceEvent::Return);
+                        let _ = self.trace_event(TraceEvent::Return, None);
                     }
                     result
                 }

Original file line number	Diff line number	Diff line change
`@@ -264,7 +264,6 @@ impl Frame {`
`264`	`264`	`if !self.locals_dirty.load(atomic::Ordering::Acquire) {`
`265`	`265`	`return Ok(());`
`266`	`266`	`}`
`267`		`- self.locals_dirty.store(false, atomic::Ordering::Release);`
`268`	`267`	`let code = &**self.code;`
`269`	`268`	`let mut fastlocals = self.fastlocals.lock();`
`270`	`269`	`for (i, &varname) in code.varnames.iter().enumerate() {`
`@@ -277,6 +276,7 @@ impl Frame {`
`277`	`276`	`Err(e) => return Err(e),`
`278`	`277`	`}`
`279`	`278`	`}`
	`279`	`+ self.locals_dirty.store(false, atomic::Ordering::Release);`
`280`	`280`	`Ok(())`
`281`	`281`	`}`
`282`	`282`
`@@ -475,7 +475,7 @@ impl ExecutingFrame<'_> {`
`475`	`475`	`&& loc.line.get() != prev_line`
`476`	`476`	`{`
`477`	`477`	`prev_line = loc.line.get();`
`478`		`- vm.trace_event(crate::protocol::TraceEvent::Line)?;`
	`478`	`+ vm.trace_event(crate::protocol::TraceEvent::Line, None)?;`
`479`	`479`	`}`
`480`	`480`	`self.update_lasti(\|i\| *i += 1);`
`481`	`481`	`let bytecode::CodeUnit { op, arg } = instructions[idx];`