refactor: review nits — lazy PhaseTimer labels and typed UnitId source accessor

Two small follow-ups from review feedback:

- `PhaseTimer::new_with(|| format!(...))` defers the label allocation
  until `PLC_TIMING` is enabled. The pipeline driver previously paid
  one `format!` per participant per stage even when timing was off,
  perturbing the measurements that the instrumentation was meant to
  observe.
- `UnitId::source_index() -> Option<usize>` replaces the
  `unit_id.is_source()` + `unit_id.raw() as usize` pair at the
  bookkeeper's closure-build site. The conversion is now encapsulated
  and non-source ids can't slip through.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ghaith

compiler/plc_driver/src/pipelines.rs

@@ -489,23 +489,23 @@ impl<T: SourceContainer> Pipeline for BuildPipeline<T> {
         let project = {
             let _t = PhaseTimer::new("pre_index (participants)");
             self.participants.iter_mut().for_each(|p| {
-                let _t = PhaseTimer::new(format!("pre_index/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("pre_index/{}", p.name()));
                 p.pre_index(&project);
             });
             self.mutable_participants.iter_mut().fold(project, |project, p| {
-                let _t = PhaseTimer::new(format!("pre_index/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("pre_index/{}", p.name()));
                 p.pre_index(project)
             })
         };
         let indexed_project = project.index(self.context.provider());
         let project = {
             let _t = PhaseTimer::new("post_index (participants)");
             self.participants.iter().for_each(|p| {
-                let _t = PhaseTimer::new(format!("post_index/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("post_index/{}", p.name()));
                 p.post_index(&indexed_project);
             });
             self.mutable_participants.iter_mut().fold(indexed_project, |project, p| {
-                let _t = PhaseTimer::new(format!("post_index/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("post_index/{}", p.name()));
                 p.post_index(project)
             })
         };
@@ -518,23 +518,23 @@ impl<T: SourceContainer> Pipeline for BuildPipeline<T> {
         let project = {
             let _t = PhaseTimer::new("pre_annotate (participants)");
             self.participants.iter().for_each(|p| {
-                let _t = PhaseTimer::new(format!("pre_annotate/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("pre_annotate/{}", p.name()));
                 p.pre_annotate(&project);
             });
             self.mutable_participants.iter_mut().fold(project, |project, p| {
-                let _t = PhaseTimer::new(format!("pre_annotate/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("pre_annotate/{}", p.name()));
                 p.pre_annotate(project)
             })
         };
         let annotated_project = project.annotate(self.context.provider());
         let mut annotated_project = {
             let _t = PhaseTimer::new("post_annotate (participants)");
             self.participants.iter().for_each(|p| {
-                let _t = PhaseTimer::new(format!("post_annotate/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("post_annotate/{}", p.name()));
                 p.post_annotate(&annotated_project);
             });
             self.mutable_participants.iter_mut().fold(annotated_project, |project, p| {
-                let _t = PhaseTimer::new(format!("post_annotate/{}", p.name()));
+                let _t = PhaseTimer::new_with(|| format!("post_annotate/{}", p.name()));
                 p.post_annotate(project)
             })
         };

compiler/plc_driver/src/pipelines/bookkeeping.rs

@@ -125,8 +125,8 @@ impl LoweringBookkeeper {
         for sig in &self.changed_signatures {
             if let Some(dependents) = reverse_deps_pre.dependents(sig) {
                 for unit_id in dependents {
-                    if unit_id.is_source() {
-                        closure.insert(unit_id.raw() as usize);
+                    if let Some(idx) = unit_id.source_index() {
+                        closure.insert(idx);
                     }
                 }
             }

compiler/plc_driver/src/pipelines/timing.rs

@@ -35,21 +35,41 @@ pub struct PhaseTimer {
 }
 
 impl PhaseTimer {
-    pub fn new(label: impl Into<String>) -> Self {
+    /// Constructs a timer with a label known at compile time. When timing is
+    /// disabled, the only work done is reading the `ENABLED` flag.
+    pub fn new(label: &'static str) -> Self {
+        Self::new_inner(active_depth(), label, enabled())
+    }
+
+    /// Constructs a timer whose label requires runtime formatting. The closure
+    /// is only invoked when timing is enabled — `format!`/allocation is
+    /// skipped on the hot path when `PLC_TIMING` is unset.
+    pub fn new_with<F>(label_fn: F) -> Self
+    where
+        F: FnOnce() -> String,
+    {
         let active = enabled();
-        let depth = if active {
-            DEPTH.with(|d| {
-                let cur = d.get();
-                d.set(cur + 1);
-                cur
-            })
-        } else {
-            0
-        };
+        let label: String = if active { label_fn() } else { String::new() };
+        Self::new_inner(active_depth(), label, active)
+    }
+
+    fn new_inner(depth: usize, label: impl Into<String>, active: bool) -> Self {
         Self { label: label.into(), start: Instant::now(), depth, active }
     }
 }
 
+fn active_depth() -> usize {
+    if enabled() {
+        DEPTH.with(|d| {
+            let cur = d.get();
+            d.set(cur + 1);
+            cur
+        })
+    } else {
+        0
+    }
+}
+
 impl Drop for PhaseTimer {
     fn drop(&mut self) {
         if !self.active {

src/index/unit_id.rs

@@ -52,6 +52,18 @@ impl UnitId {
         self.0 < Self::UNTAGGED.0
     }
 
+    /// Returns the source-list index if this is a source unit, or `None`
+    /// for built-in, synthetic, or untagged ids. Prefer this over
+    /// `is_source()` + `raw() as usize`: the conversion is encapsulated and
+    /// non-source ids can't slip through silently.
+    pub fn source_index(self) -> Option<usize> {
+        if self.is_source() {
+            Some(self.0 as usize)
+        } else {
+            None
+        }
+    }
+
     pub fn is_builtin(self) -> bool {
         self == Self::BUILTIN
     }