Refactor/printer (#122)

This PR splits `src/printer.rs` (~1700 lines) into a `src/printer/`
directory module with focused submodules, then follows up with doc
comments, idiomatic Rust cleanup, and a structural improvement to the
phase boundary enforcement.

All integration and UI tests pass unchanged.

## Structure

The split creates nine submodules, each with a single responsibility:

| Module | Lines | Responsibility |
|--------|------:|----------------|
| `schema.rs` | ~510 | Data model types (`SmirJson`, `Item`,
`AllocInfo`, `TypeMetadata`, `LinkMapKey`, etc.) |
| `mir_visitor.rs` | ~430 | `BodyAnalyzer`: single-pass MIR body
traversal collecting calls, allocs, types, spans |
| `collect.rs` | ~230 | Three-phase pipeline: collect items, analyze
bodies, assemble final output |
| `items.rs` | ~225 | `Item` construction from `MonoItem` with optional
debug-level details |
| `types.rs` | ~170 | `TypeMetadata` construction from `TyKind` + layout
|
| `ty_visitor.rs` | ~120 | Recursive type visitor collecting all
reachable types with layout info |
| `link_map.rs` | ~107 | Link-time function resolution map (type +
instance kind to symbol name) |
| `mod.rs` | ~83 | Module glue: macros, env-var helpers, re-exports,
`emit_smir` |
| `util.rs` | ~64 | Name resolution, attribute queries, small helpers |

## Changes by commit

1. **Split printer.rs into a directory module**: mechanical extraction;
no code changes beyond moving `use` / `extern crate` declarations into
each submodule and adjusting visibility.

2. **Add module-level doc comments to all printer submodules**: `//!`
doc headers describing each module's purpose and key types.

3. **Idiomatic Rust cleanup across printer submodules**:
   - `if let Some(x) = y { x } else { z }` to `y.unwrap_or_else(|| z)`
- `if opt.is_none() { return; } let val = opt.unwrap()` to `let
Some(val) = opt else { return; }`
- Remove unnecessary `.clone()` calls on `key` in `get_mut` / `insert`
- Change `collect_alloc` to take `offset: usize` by value (was `&usize`)
- Extract `opaque_placeholder_ty()` helper for the `Ty::to_val(0)`
sentinel
- Doc comments on `FnSymType`, `LinkMapKey`, `AllocInfo`,
`TypeMetadata`, `SourceData`, `SmirJson`, `SmirJsonDebugInfo`

4. **Address review feedback and fix doc comment inaccuracies**.

5. **Remove `MonoItem` from `Item` for structural phase enforcement**:

The three-phase collection pipeline has an invariant: phase 3
(`assemble_smir`) must not call `inst.body()` or otherwise re-enter
rustc. Previously this was enforced by convention only, because `Item`
carried a `pub(super) mono_item: MonoItem` field that gave any code with
an `&Item` full access to `Instance::body()`.

This commit removes `mono_item` from `Item` entirely, so the invariant
is structural: phase 3 code literally cannot call `inst.body()` because
it never has a `MonoItem` to call it on. The type system enforces what
discipline previously had to.

Concretely: `mk_item` now returns `(MonoItem, Item)` instead of `Item`,
and the phase 1+2 maps carry `(MonoItem, Item)` tuples. The `MonoItem`
lives alongside the `Item` during collection and analysis, gets passed
directly to `maybe_add_to_link_map` and `warn_missing_body` (now a free
function in `collect.rs`), and is naturally dropped when only the `Item`
is pushed into `all_items`. By the time `assemble_smir` runs, no
`MonoItem` values exist.

As a bonus, this fixes a latent `PartialEq`/`Ord` inconsistency:
`PartialEq` was comparing `mono_item` while `Ord` compared `symbol_name`
+ item-kind name. `PartialEq` now delegates to `Ord`, so the two are
consistent.

## Test plan

- [x] `cargo build` succeeds
- [x] `cargo clippy` clean
- [x] `make integration-test` passes (28/28)
- [x] `make test-ui` passes (2875/2875, 0 failures)

Author: cds-amal

CHANGELOG.md

@@ -22,21 +22,23 @@ retroactive best-effort summary; earlier changes were not formally tracked.
 - `cargo_stable_mir_json` helper binary for cargo integration ([#47])
 - Nix derivation for reproducible builds ([#96])
 - macOS support ([#97])
+- Mutability field on `PtrType` and `RefType` in `TypeMetadata`, needed to distinguish `PtrToPtr` casts that change mutability from those that change pointee type ([#127])
+- ADR-001: index-first graph architecture for MIR visualization ([#124])
 
 ### Changed
-- Restructured `printer.rs` into a declarative 3-phase pipeline: `collect_items` -> `collect_and_analyze_items` -> `assemble_smir`, with `CollectedCrate`/`DerivedInfo` interface types enforcing the boundary between collection and assembly
-- Added `AllocMap` with debug-mode coherence checking (`verify_coherence`): asserts that every `AllocId` in stored bodies exists in the alloc map and that no body is walked twice; zero cost in release builds
-- Removed dead static-item fixup from `assemble_smir` (violated the phase boundary, misclassified statics as functions; never triggered in integration tests)
-- Rewrote `run_ui_tests.sh`: flag-based CLI (`--verbose`, `--save-generated-output`, `--save-debug-output`), build-once-then-invoke (eliminates per-test cargo overhead), arch-aware skip logic for cross-platform test lists
-- UI test runners now extract and pass `//@ compile-flags:`, `//@ edition:`, and `//@ rustc-env:` directives from test files (previously silently ignored)
+- Restructured `printer.rs` into a declarative 3-phase pipeline: `collect_items` -> `collect_and_analyze_items` -> `assemble_smir`, with `CollectedCrate`/`DerivedInfo` interface types enforcing the boundary between collection and assembly ([#121])
+- Added `AllocMap` with debug-mode coherence checking (`verify_coherence`): asserts that every `AllocId` in stored bodies exists in the alloc map and that no body is walked twice; zero cost in release builds ([#121])
+- Removed dead static-item fixup from `assemble_smir` (violated the phase boundary, misclassified statics as functions; never triggered in integration tests) ([#121])
+- Rewrote `run_ui_tests.sh`: flag-based CLI (`--verbose`, `--save-generated-output`, `--save-debug-output`), build-once-then-invoke (eliminates per-test cargo overhead), arch-aware skip logic for cross-platform test lists ([#126])
+- UI test runners now extract and pass `//@ compile-flags:`, `//@ edition:`, and `//@ rustc-env:` directives from test files (previously silently ignored) ([#126])
 - Switched from compiler build to `rustup`-managed toolchain ([#33])
 - Removed forked rust dependency ([#19])
 
 ### Fixed
-- Fixed `get_prov_ty` to recursively walk nested struct/tuple fields when resolving provenance types; previously used exact byte-offset matching which panicked on pointers inside nested structs (e.g., `&str` at offset 56 inside `TestDesc` inside `TestDescAndFn`)
-- Removed incorrect `builtin_deref` assertions from VTable and Static allocation collection that rejected valid non-pointer types (raw `*const ()` vtable pointers, non-pointer statics)
-- Replaced panicking `unwrap`/`assert` calls in `get_prov_ty` with graceful fallbacks for layout failures, non-rigid types, and unexpected offsets
-- Fixed early `return` in `BodyAnalyzer::visit_terminator` that skipped `super_terminator()`, causing alloc/type/span collection to miss everything inside `Call` terminators with non-`ZeroSized` function operands (const-evaluated function pointers); bug present since [`aff2dd0`](https://github.com/runtimeverification/stable-mir-json/commit/aff2dd0)
+- Fixed `get_prov_ty` to recursively walk nested struct/tuple fields when resolving provenance types; previously used exact byte-offset matching which panicked on pointers inside nested structs (e.g., `&str` at offset 56 inside `TestDesc` inside `TestDescAndFn`) ([#126])
+- Removed incorrect `builtin_deref` assertions from VTable and Static allocation collection that rejected valid non-pointer types (raw `*const ()` vtable pointers, non-pointer statics) ([#126])
+- Replaced panicking `unwrap`/`assert` calls in `get_prov_ty` with graceful fallbacks for layout failures, non-rigid types, and unexpected offsets ([#126])
+- Fixed early `return` in `BodyAnalyzer::visit_terminator` that skipped `super_terminator()`, causing alloc/type/span collection to miss everything inside `Call` terminators with non-`ZeroSized` function operands (const-evaluated function pointers); bug present since [`aff2dd0`](https://github.com/runtimeverification/stable-mir-json/commit/aff2dd0) ([#126])
 - Avoided duplicate `inst.body()` calls that were reallocating `AllocId`s ([#120])
 - Prevented svg/png generation when `dot` is unavailable ([#117])
 - Removed unreachable early return in D2 legend rendering ([#118])
@@ -72,6 +74,10 @@ retroactive best-effort summary; earlier changes were not formally tracked.
 [#117]: https://github.com/runtimeverification/stable-mir-json/pull/117
 [#118]: https://github.com/runtimeverification/stable-mir-json/pull/118
 [#120]: https://github.com/runtimeverification/stable-mir-json/pull/120
+[#121]: https://github.com/runtimeverification/stable-mir-json/pull/121
+[#124]: https://github.com/runtimeverification/stable-mir-json/pull/124
+[#126]: https://github.com/runtimeverification/stable-mir-json/pull/126
+[#127]: https://github.com/runtimeverification/stable-mir-json/pull/127
 
 ## [0.1.0] - 2024-11-29
 

src/mk_graph/index.rs

@@ -75,6 +75,7 @@ pub enum TypeKind {
         pointee: Ty,
         mutability: stable_mir::mir::Mutability,
     },
+    Dyn,
     Function,
     Void,
 }
@@ -409,6 +410,10 @@ impl TypeEntry {
                     layout_info,
                 )
             }
+            TypeMetadata::DynType { name, layout } => {
+                let layout_info = layout.as_ref().map(LayoutInfo::from_shape);
+                (name.clone(), TypeKind::Dyn, layout_info)
+            }
             TypeMetadata::FunType(name) => (name.clone(), TypeKind::Function, None),
             TypeMetadata::VoidType => ("()".to_string(), TypeKind::Void, None),
         };