Panics · P20 Debug: stale selections, frames and focus paths mark none; the timeline's wheel keeps a drawable range

src/debug/flame.rs

@@ -37,11 +37,17 @@ pub struct Placed {
     pub samples: u64,
 }
 
-/// The frames under `focus`, a path of call indices, each spanning its share of the samples; `focus` fills the width and its callers stack above it, full width.
+/// The frames under `focus`, a path of call indices kept as far as the profile holds it, each spanning its share of the samples; `focus` fills the width and its callers stack above it, full width.
 pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec<Placed> {
     let mut out = Vec::new();
     let mut frame = root;
+    let mut kept = focus.len();
     for (depth, ix) in focus.iter().enumerate() {
+        let Some(call) = frame.calls.get(*ix) else {
+            log::error!("flame graph: focus {focus:?} leaves the profile at depth {depth}");
+            kept = depth;
+            break;
+        };
         out.push(Placed {
             path: focus[..depth].to_vec(),
             name: frame.name.clone(),
@@ -50,7 +56,7 @@ pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec<Placed> {
             end: 1.0,
             samples: frame.total(),
         });
-        frame = &frame.calls[*ix];
+        frame = call;
     }
     fn under(
         frame: &ProfileFrame,
@@ -78,7 +84,7 @@ pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec<Placed> {
             at += width;
         }
     }
-    under(frame, focus.to_vec(), focus.len(), 0.0, 1.0, &mut out);
+    under(frame, focus[..kept].to_vec(), kept, 0.0, 1.0, &mut out);
     out
 }
 
@@ -247,11 +253,17 @@ impl RenderOnce for Flamegraph {
                             move |bounds, _, window, cx| {
                                 let pixels = text_size.to_pixels(window.rem_size());
                                 let face = gpui::font(font.clone());
-                                let advance = window
-                                    .text_system()
-                                    .advance(window.text_system().resolve_font(&face), pixels, 'm')
-                                    .expect("the code font has an m")
-                                    .width;
+                                let resolved = window.text_system().resolve_font(&face);
+                                let advance =
+                                    match window.text_system().advance(resolved, pixels, 'm') {
+                                        Ok(advance) => advance.width,
+                                        Err(error) => {
+                                            log::error!(
+                                                "flame graph: no advance for its font: {error:#}"
+                                            );
+                                            pixels * 0.6
+                                        }
+                                    };
                                 for frame in paint_placed.iter() {
                                     let x0 = bounds.origin.x + bounds.size.width * frame.start;
                                     let width = bounds.size.width * (frame.end - frame.start);
@@ -287,7 +299,7 @@ impl RenderOnce for Flamegraph {
                                             underline: None,
                                             strikethrough: None,
                                         };
-                                        window
+                                        let painted = window
                                             .text_system()
                                             .shape_line(label.into(), pixels, &[run], None)
                                             .paint(
@@ -301,8 +313,12 @@ impl RenderOnce for Flamegraph {
                                                 None,
                                                 window,
                                                 cx,
-                                            )
-                                            .expect("a flame label paints");
+                                            );
+                                        if let Err(error) = painted {
+                                            log::error!(
+                                                "flame graph: a label failed to paint: {error:#}"
+                                            );
+                                        }
                                     }
                                 }
                             },

src/debug/hex.rs

@@ -62,18 +62,26 @@ impl HexViewer {
         }
     }
 
-    /// The address of the first byte.
+    /// The address of the first byte; the last must fit an address too.
     pub fn base(mut self, base: u64) -> Self {
+        let last = self.bytes.len().saturating_sub(1) as u64;
+        assert!(
+            base.checked_add(last).is_some(),
+            "a dump at {base:#X} runs past the last address"
+        );
         self.base = base;
         self
     }
 
     pub fn selected(mut self, bytes: Range<usize>) -> Self {
-        assert!(
-            bytes.end <= self.bytes.len(),
-            "a selection lies inside the bytes"
-        );
-        self.selected = Some(bytes);
+        let inside = bytes.start < bytes.end && bytes.end <= self.bytes.len();
+        if !inside {
+            log::error!(
+                "hex viewer: no bytes {bytes:?} in {}; none selected",
+                self.bytes.len()
+            );
+        }
+        self.selected = inside.then_some(bytes);
         self
     }
 
@@ -90,9 +98,12 @@ impl RenderOnce for HexViewer {
         let theme = cx.theme();
         let colors = theme.colors.clone();
         let rows = self.bytes.len().div_ceil(WIDTH);
-        let digits = format!("{:X}", self.base + self.bytes.len() as u64)
-            .len()
-            .max(8);
+        let digits = format!(
+            "{:X}",
+            self.base + self.bytes.len().saturating_sub(1) as u64
+        )
+        .len()
+        .max(8);
         let reading = self
             .selected
             .as_ref()
@@ -198,6 +209,15 @@ impl RenderOnce for HexViewer {
 mod tests {
     use super::*;
 
+    #[test]
+    fn a_selection_off_the_bytes_selects_none() {
+        let viewer = || HexViewer::new("hex", vec![1, 2, 3]);
+        let (inside, at_end, past) = (0..2, 3..3, 2..9);
+        assert_eq!(viewer().selected(inside.clone()).selected, Some(inside));
+        assert_eq!(viewer().selected(at_end).selected, None);
+        assert_eq!(viewer().selected(past).selected, None);
+    }
+
     #[test]
     fn bytes_read_as_numbers_and_text() {
         let bytes = [0x2A, 0x01, 0x00, 0x00, 0xFF];

src/debug/stack.rs

@@ -71,10 +71,12 @@ impl CallStack {
     }
 
     pub fn current(mut self, frame: usize) -> Self {
-        assert!(
-            frame < self.frames.len(),
-            "the current frame is on the stack"
-        );
+        if frame >= self.frames.len() {
+            log::error!(
+                "call stack: no frame {frame} of {}; none marked",
+                self.frames.len()
+            );
+        }
         self.current = frame;
         self
     }

src/debug/tests.rs

@@ -138,3 +138,112 @@ fn tab_reaches_each_span_and_enter_picks_it(cx: &mut TestAppContext) {
         "the second Tab stands on the second span"
     );
 }
+
+/// A stack whose current frame left with a shorter stack.
+struct Unwound;
+
+impl Render for Unwound {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        let frame = super::StackFrame {
+            function: "main".into(),
+            path: "src/main.rs".into(),
+            line: 3,
+            library: false,
+        };
+        div()
+            .w(px(400.0))
+            .child(super::CallStack::new("stack", [frame]).current(4))
+    }
+}
+
+#[gpui::test]
+fn a_current_frame_past_the_stack_marks_none(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let (_, cx) = cx.add_window_view(|_, _| Unwound);
+    settle(cx);
+}
+
+/// A flame graph focused on a call its new profile lacks; a dump ending at the last address.
+struct Edges(bool);
+
+impl Render for Edges {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        let root = super::ProfileFrame {
+            name: "root".into(),
+            own: 1,
+            calls: vec![],
+        };
+        div().w(px(400.0)).h(px(100.0)).child(match self.0 {
+            true => super::Flamegraph::new("flame", root)
+                .focus(vec![0])
+                .into_any_element(),
+            false => super::HexViewer::new("hex", vec![0x42])
+                .base(u64::MAX)
+                .into_any_element(),
+        })
+    }
+}
+
+#[gpui::test]
+fn a_lost_focus_and_the_last_address_draw(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    for flame in [true, false] {
+        let (_, cx) = cx.add_window_view(move |_, _| Edges(flame));
+        settle(cx);
+    }
+}
+
+/// A timeline whose owner keeps each range it reports.
+struct Zooming((f64, f64));
+
+impl Render for Zooming {
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let owner = cx.entity();
+        div().w(px(400.0)).child(
+            TimelineProfiler::new("timeline", ["Main"], vec![], self.0).on_range(
+                move |range, _, cx| {
+                    owner.update(cx, |owner, cx| {
+                        owner.0 = range;
+                        cx.notify();
+                    })
+                },
+            ),
+        )
+    }
+}
+
+#[gpui::test]
+fn a_zoom_past_the_finest_range_stays_put(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let start = (1.0, 1.0 + f64::EPSILON);
+    let (owner, cx) = cx.add_window_view(move |_, _| Zooming(start));
+    settle(cx);
+    for _ in 0..2 {
+        cx.simulate_event(gpui::ScrollWheelEvent {
+            position: point(px(250.0), px(30.0)),
+            delta: gpui::ScrollDelta::Pixels(point(px(0.0), px(400.0))),
+            modifiers: Modifiers::none(),
+            touch_phase: gpui::TouchPhase::Moved,
+        });
+        settle(cx);
+    }
+    let range = owner.read_with(cx, |owner, _| owner.0);
+    assert!(range.1 > range.0, "{range:?}");
+}
+
+#[gpui::test]
+fn a_wheel_too_large_for_a_zoom_stays_put(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let (owner, cx) = cx.add_window_view(|_, _| Zooming((0.0, 25.0)));
+    settle(cx);
+    for pixels in [300_000.0, -300_000.0] {
+        cx.simulate_event(gpui::ScrollWheelEvent {
+            position: point(px(250.0), px(30.0)),
+            delta: gpui::ScrollDelta::Pixels(point(px(0.0), px(pixels))),
+            modifiers: Modifiers::none(),
+            touch_phase: gpui::TouchPhase::Moved,
+        });
+        settle(cx);
+    }
+    assert_eq!(owner.read_with(cx, |owner, _| owner.0), (0.0, 25.0));
+}

src/debug/timeline.rs

@@ -187,13 +187,21 @@ impl RenderOnce for TimelineProfiler {
                         let (x, y) = (f64::from(f32::from(delta.x)), f64::from(f32::from(delta.y)));
                         cx.stop_propagation();
                         let width = f64::from(f32::from(bounds.size.width)).max(1.0);
-                        if x.abs() > y.abs() {
+                        let next = if x.abs() > y.abs() {
                             let shift = -x / width * length;
-                            on_range((from + shift, to + shift), window, cx);
+                            Some((from + shift, to + shift))
                         } else {
                             let pointer =
                                 f64::from(f32::from(event.position.x - bounds.origin.x)) / width;
-                            on_range(zoomed((from, to), pointer, (-y / ZOOM).exp()), window, cx);
+                            let factor = (-y / ZOOM).exp();
+                            (factor > 0.0 && factor.is_finite())
+                                .then(|| zoomed((from, to), pointer, factor))
+                        };
+                        let fits = |next: &(f64, f64)| {
+                            next.0.is_finite() && next.1.is_finite() && next.1 > next.0
+                        };
+                        if let Some(next) = next.filter(fits) {
+                            on_range(next, window, cx);
                         }
                     })
                     .children(self.tracks.iter().enumerate().map(|(track, name)| {

tasks/panics.md

@@ -34,7 +34,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca
 - [x] P17 Code Editor — `src/editor`
 - [x] P18 Terminal — `src/terminal`
 - [x] P19 Git — `src/git`
-- [ ] P20 Debug — `src/debug`
+- [x] P20 Debug — `src/debug`
 - [ ] P21 Documents — `src/documents`
 - [ ] P22 Collaboration — `src/collab`
 - [ ] P23 AI Chat — `src/chat`
@@ -85,3 +85,4 @@ One line per item: what changed, what stayed and why, review rounds.
 - P17: the editor keeps every owner offset on whole characters of its text (checked on intake, carried through edits by `editor::anchors`, cleared when the whole text changes); stale selections and edit batches are dropped; a painted row whose line the text lost takes no click, drag or IME query; the sticky header reads this frame; a glob too big to compile matches nothing; a missing font advance logs. Kept: owner contracts and construction. Review: 5 rounds, passed on the fifth.
 - P18: row paints and the font advance log gpui faults; a pty side caps at what ConPTY and alacritty's pixel sizes hold; a default shell past the list keeps the one before. Kept: constant regexes, captured matches, keyed rows, non-empty shells. Review: 2 rounds.
 - P19: a blame that lags its code draws every line, bare where no owner fits; `git::resolve` returns `Option` for a conflict gone from the text. Kept: diff groups, merge regions, similar's pinned resolutions. Review: 2 rounds.
+- P20: a hex selection off the bytes and a stack frame past the stack select none; a flame focus keeps what the profile holds; the timeline's wheel reports only finite, positive ranges from finite factors; a dump's last byte must have an address; flame paints log. Kept: disassembly patterns, `zoomed`'s contract. Review: 3 rounds.