Factor: collapse triplicated alloc recovery into one arm (#135)

This is a follow-up to #131. That PR fixed a real panic
(non-builtin-deref Static/VTable allocations), but the fix landed as
three nearly identical match arms: Static, VTable, and Function, each
repeating the same "try `get_prov_ty`, fall back to opaque placeholder"
logic. The only meaningful difference between them was a single
predicate ("is this type usable directly?") and the debug log string.
This PR collapses them back into one arm, makes the predicate explicit,
and documents a previously unexplained jq filter.

## What changed

**`src/printer/mir_visitor.rs`**

The three arms now share a single code path. The predicate that actually
differs between them is captured in a `needs_recovery` variable:

- Static/VTable: `builtin_deref(true).is_none()` (not a reference, raw
pointer, or Box)
- Function: `!kind.is_fn_ptr()` (outer type isn't already a function
pointer)

Worth noting: these two predicates are *not* equivalent (a function
pointer passes `builtin_deref` but fails `is_fn_ptr`), which is why a
naive "just combine the arms" without the inner match would have changed
behavior for Function allocs. The inner match on `global_alloc` makes
this asymmetry visible rather than hiding it across separate arms.

The rest of the logic (try `get_prov_ty`, fall back to opaque
placeholder) is written once. Also dropped an unnecessary `.clone()` on
the `builtin_deref` call; it takes `&self`.

**`tests/integration/normalise-filter.jq`**

The `def_id` filter had a terse comment ("unrelated to this regression")
that didn't explain *why* it exists or why it's safe to strip globally.
Replaced it with a proper explanation: `def_id` values are interned
compiler indices (same class as `alloc_id` and `adt_def`) that are
consistent within a single rustc invocation but non-deterministic across
runs. Downstream consumers need them as cross-reference keys to join
`AggregateKind::Adt` in MIR bodies with type metadata entries, so they
can't be dropped from the output itself; we only strip them here for
golden-file comparison. (See #125 for the full picture on interned-index
non-determinism.)

## Test plan

- [ ] `cargo build` compiles cleanly
- [ ] `make integration-test` passes (no behavioral change; this is a
pure refactor)

Author: cds-amal

src/printer/mir_visitor.rs

@@ -265,12 +265,21 @@ fn collect_alloc(
                     .insert(val, (opaque_placeholder_ty(), global_alloc.clone()));
             }
         }
-        GlobalAlloc::Static(_) => {
-            // Keep builtin-deref behavior; recover only non-builtin-deref cases.
-            if kind.clone().builtin_deref(true).is_none() {
+        GlobalAlloc::Static(_) | GlobalAlloc::VTable(_, _) | GlobalAlloc::Function(_) => {
+            // Does the outer type need provenance recovery to find the real
+            // pointee? Static/VTable pointers are usable directly when
+            // builtin_deref succeeds; Function allocs are usable directly
+            // when the outer type is already a fn pointer.
+            let needs_recovery = match &global_alloc {
+                GlobalAlloc::Function(_) => !kind.is_fn_ptr(),
+                _ => kind.builtin_deref(true).is_none(),
+            };
+
+            if needs_recovery {
                 let prov_ty = get_prov_ty(ty, &offset);
                 debug_log_println!(
-                    "DEBUG: GlobalAlloc::Static with non-builtin-deref type; alloc_id={:?}, ty={:?}, offset={}, kind={:?}, recovered_prov_ty={:?}",
+                    "DEBUG: {:?} with non-direct type; alloc_id={:?}, ty={:?}, offset={}, kind={:?}, recovered_prov_ty={:?}",
+                    global_alloc,
                     val,
                     ty,
                     offset,
@@ -288,62 +297,7 @@ fn collect_alloc(
                         .insert(val, (opaque_placeholder_ty(), global_alloc.clone()));
                 }
             } else {
-                val_collector
-                    .visited_allocs
-                    .insert(val, (ty, global_alloc.clone()));
-            }
-        }
-        GlobalAlloc::VTable(_, _) => {
-            // Same policy as Static: keep builtin-deref, recover non-builtin-deref.
-            if kind.clone().builtin_deref(true).is_none() {
-                let prov_ty = get_prov_ty(ty, &offset);
-                debug_log_println!(
-                    "DEBUG: GlobalAlloc::VTable with non-builtin-deref type; alloc_id={:?}, ty={:?}, offset={}, kind={:?}, recovered_prov_ty={:?}",
-                    val,
-                    ty,
-                    offset,
-                    kind,
-                    prov_ty
-                );
-                if let Some(p_ty) = prov_ty {
-                    val_collector
-                        .visited_allocs
-                        .insert(val, (p_ty, global_alloc.clone()));
-                } else {
-                    // Unknown is safer than wrong pointee type.
-                    val_collector
-                        .visited_allocs
-                        .insert(val, (opaque_placeholder_ty(), global_alloc.clone()));
-                }
-            } else {
-                val_collector
-                    .visited_allocs
-                    .insert(val, (ty, global_alloc.clone()));
-            }
-        }
-        GlobalAlloc::Function(_) => {
-            if !kind.is_fn_ptr() {
-                let prov_ty = get_prov_ty(ty, &offset);
-                debug_log_println!(
-                    "DEBUG: GlobalAlloc::Function with non-fn-ptr type; alloc_id={:?}, ty={:?}, offset={}, kind={:?}, recovered_prov_ty={:?}",
-                    val,
-                    ty,
-                    offset,
-                    kind,
-                    prov_ty
-                );
-                if let Some(p_ty) = prov_ty {
-                    val_collector
-                        .visited_allocs
-                        .insert(val, (p_ty, global_alloc.clone()));
-                } else {
-                    // Could not recover a precise pointee type; use an opaque 0-valued Ty
-                    // as a conservative placeholder.
-                    val_collector
-                        .visited_allocs
-                        .insert(val, (opaque_placeholder_ty(), global_alloc.clone()));
-                }
-            } else {
+                // Type is already the correct pointee; use it directly.
                 val_collector
                     .visited_allocs
                     .insert(val, (ty, global_alloc.clone()));

tests/integration/normalise-filter.jq

@@ -29,5 +29,12 @@
     ( .types | map(select(.[0].FunType) | sort) )
   ] | flatten(1) )
 }
-# drop unstable compiler-internal ids that are unrelated to this regression
+# Strip def_id fields globally. These are interned compiler indices (the
+# underlying ID inside AdtDef) that are consistent within a single rustc
+# invocation but not stable across runs; the same non-determinism that
+# affects alloc_id, Ty indices, and adt_def (see lines 5-6, 18-21 above).
+# Downstream consumers use adt_def/def_id as cross-reference keys to join
+# AggregateKind::Adt in MIR bodies with type metadata entries, so the
+# values can't be dropped from the output itself; we only strip them here
+# for golden-file comparison.
 | walk(if type == "object" then del(.def_id) else . end)