Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/stdlib/src/_opcode.rs`:
- Around line 314-326: The disassembly capture currently assumes dis.dis()
returns a string and panics when it returns None; update the dis_output function
so inside interp.enter (the closure passed to interp.enter) you compile the
source as before, create an io.StringIO via vm.import("io") and
io.get_attr("StringIO"), replace sys.stdout by setting sys.stdout to that
StringIO (save the previous stdout), call dis = dis_module.get_attr("dis") and
invoke dis.call((pycode,), vm) checking the call result, restore sys.stdout to
the saved value, then read the captured text via
string_io.call_method("getvalue", (), vm)?.try_into_value::<String>(vm) and
return that String (propagate errors with ?); also simplify the dis_output
signature to fn dis_output(source: &str) -> String (or -> PyResult<String>
inside enter) instead of Result<(), ()>, and remove reliance on
insta::internals::AutoName by switching to the public insta naming API.
---
Nitpick comments:
In `@crates/stdlib/src/_opcode.rs`:
- Around line 309-331: The function dis_output currently returns Result<(), ()>
with a final Ok(()) and reborrows source; simplify it by changing the signature
of dis_output to return () (unit), remove the final Ok(()) return, and stop
reborrowing source when calling vm.compile (use source instead of &source); keep
the existing interp.enter closure and its internal error handling as-is (the
closure still returns PyResult<String> and unwraps).
- Around line 310-312: The tests rebuild an Interpreter for every dis_output
#[test_case], slowing test runs; create and reuse a shared static interpreter
(e.g., a shared_interp function backed by std::sync::OnceLock or LazyLock that
initializes vm::Interpreter via vm::Interpreter::builder and
crate::stdlib_module_defs once) and call shared_interp().enter(|vm| { … })
inside dis_output instead of rebuilding; verify whether the VM/VirtualMachine
types are Sync/Send—if not, either run those tests serially (use serial_test or
#[test] with single-threaded execution) or restrict the static to
single-threaded use to avoid data races.
- Line 328: Replace the unstable internals usage in the snapshot assertion:
remove the first argument `insta::internals::AutoName` from the
`insta::assert_snapshot!` invocation so it uses the public simplified API (i.e.,
call `insta::assert_snapshot!(output, source)`), leaving the auto-naming
behavior driven by the test_case description intact.