interp: correct four comments about why the frame virtualizable forces exist

pyre/pyre-interpreter/src/builtins.rs

@@ -11154,12 +11154,15 @@ fn builtin_globals(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError>
 /// (`interp_inspect.py:7-11 locals` — `ec.gettopframe_nohidden()`).
 ///
 /// Going through `gettopframe_nohidden` is what makes the frame's fastlocals
-/// readable: it runs `force_frame` on every frame it walks
-/// (`executioncontext.rs:409-421`), and `fast2locals` reads
-/// `locals_cells_stack_w` directly, so an unforced virtualizable hands back an
-/// array of nulls — which `fast2locals` renders as an EMPTY mapping rather
-/// than a stale one.  Reading `CURRENT_FRAME` instead skips that force, and
-/// under the JIT the caller then sees no locals at all.
+/// readable, but the mechanism is the VREF force on its first line, not a
+/// per-frame `force_frame`: the walk itself only follows `f_backref` and tests
+/// `hide()`.  That materializes the top frame and nothing below it, which is
+/// exactly what this consumer needs — `locals()` reports on the top frame.
+/// `getdictscope` reads `locals_cells_stack_w` directly, so an unforced
+/// virtualizable hands back an array of nulls, which `fast2locals` renders as
+/// an EMPTY mapping rather than a stale one.  Reading `CURRENT_FRAME` instead
+/// skips the vref force, and under the JIT the caller then sees no locals at
+/// all.
 fn topframe_for_locals() -> *mut crate::PyFrame {
     let ec = crate::call::getexecutioncontext() as *mut crate::PyExecutionContext;
     if ec.is_null() {

pyre/pyre-interpreter/src/executioncontext.rs

@@ -33,14 +33,30 @@ pub fn force_frame(frame: *mut PyFrame) {
 
 /// Force a frame whose fastlocals application code is about to read.
 ///
-/// RPython needs no such call: storing the frame pointer anywhere the JIT
-/// cannot see forces the virtualizable by escape analysis, so
-/// `pyframe.py:539 fast2locals` always finds a materialized
-/// `locals_cells_stack_w`.  Pyre forces through explicit hooks instead, and
-/// [`PyExecutionContext::gettopframe_nohidden`] only covers the frames IT
-/// walks — a frame handed out some other way (a traceback's `tb_frame`)
-/// reaches `fast2locals` unforced, whose null slots render as an EMPTY
-/// mapping rather than a stale one.
+/// RPython needs no such call because the force is injected for it, not
+/// because none happens: `rvirtualizable.py:49-53 hook_access_field` genops
+/// `jit_force_virtualizable` on every redirected FIELD access,
+/// `virtualizable.py:288-292` rewrites those into a
+/// `force_virtualizable_if_necessary` call across `translator.graphs`, and
+/// `jtransform.py:2164-2172 rewrite_op_jit_force_virtualizable` drops it again
+/// only in the graphs the codewriter looks inside.  So `pyframe.py:539
+/// fast2locals` always finds a materialized `locals_cells_stack_w` with no
+/// hand-placed hook anywhere.
+///
+/// Pyre's rtyper declines to build a virtualizable `InstanceRepr` at all
+/// (`rclass.rs buildinstancerepr`, blocked on `FieldListAccessor` /
+/// `_parse_field_list` parity), so that injection does not exist here and
+/// explicit calls like this one stand in for it — load-bearing, not redundant.
+/// Deleting them was measured: `f_lineno` starts reporting the `def` line,
+/// `f_locals` grows an entry, and the two read together segfault, with the
+/// whole synthetic corpus green throughout.
+///
+/// [`PyExecutionContext::gettopframe_nohidden`] does not substitute.  It forces
+/// the VREF of the frame it starts from and then walks `f_backref` unforced, so
+/// it materializes the top frame only — enough for `locals()`, which reports on
+/// that frame, and nothing for a frame handed out some other way (a traceback's
+/// `tb_frame`), which reaches `fast2locals` unforced; its null slots render as
+/// an EMPTY mapping rather than a stale one.
 ///
 /// # Safety
 /// `frame` must be a live `PyFrame` (or null).

pyre/pyre-interpreter/src/module/sys/vm.rs

@@ -566,10 +566,14 @@ fn simple_namespace_replace(args: &[PyObjectRef]) -> crate::PyResult {
 /// (`ExecutionContext::gettopframe_raw`), so `gettopframe_nohidden` is not
 /// "force-free" — it is free of the *virtualizable* force that
 /// `gettopframe` adds. Upstream takes neither of those, because
-/// `look_inside_iff(isconstant(depth))` traces a constant `depth` THROUGH: the
-/// walk never becomes a residual call, and the frame it hands to app level
-/// materialises by escape analysis instead
-/// (`executioncontext::force_frame_before_locals_read` states that contract).
+/// `look_inside_iff(isconstant(depth))` traces a constant `depth` THROUGH, so
+/// the walk never becomes a residual call — and the frame it hands to app level
+/// materialises through the force `rvirtualizable.py:49-53 hook_access_field`
+/// injects at every redirected FIELD access, which
+/// `jtransform.py:2164-2172 rewrite_op_jit_force_virtualizable` deletes only in
+/// the graphs the codewriter looks inside.  Pyre has no such injection —
+/// `rclass.rs buildinstancerepr` declines a virtualizable `InstanceRepr`
+/// outright — so these two calls ARE that mechanism, relocated to one consumer.
 /// Pyre calls this residually, so both forces are load-bearing here:
 /// `gettopframe()` because a JIT-inlined callee has no frame of its own until a
 /// force materialises one — an unforced walk would start at the caller and then

pyre/pyre-interpreter/src/typedef.rs

@@ -7211,9 +7211,11 @@ fn init_frame_type(ns: PyObjectRef) {
             // Both arms end at `fast2locals` (the proxy routes its reads
             // back through the frame), which reads `locals_cells_stack_w`
             // directly — so the virtualizable has to be materialized first or
-            // the mapping comes back EMPTY.  `sys._getframe` gets that for
-            // free from `gettopframe_nohidden`; a frame reached through a
-            // traceback's `tb_frame` does not.
+            // the mapping comes back EMPTY.  `sys._getframe` materializes it
+            // with its own explicit `force_frame` (`module/sys/vm.rs
+            // getframe`), not as a side effect of the walk: `gettopframe_nohidden`
+            // forces only the VREF of the frame it starts from.  A frame
+            // reached through a traceback's `tb_frame` gets neither.
             crate::executioncontext::force_frame_before_locals_read(f);
             let frame = unsafe { &mut *f };
             if frame.code().flags.contains(crate::CodeFlags::OPTIMIZED) {