Cover joining two expressions, and print a request one way

Four findings over 05e6ced, which no earlier round reviewed.

`SizeRequest::join` grafts the other side's nodes into this side's arena,
which happens only when both sides are expressions -- nothing in the suite
did that, so the whole thing passed with a `panic!` in that arm. It is
where a missed renumbering would be silent, since an operand copied
without remapping still names a node that exists. A fixture at two window
widths with absolute geometry covers it now, and the deferred corpus puts
an expression on both sides of one arm; both were checked to reach it by
instrumenting again. The path was already right.

`SizeRequest` grew a `Display` because a derived `Debug` of an arena is
not a tree anyone can write out again, and `describe` moved onto it -- but
`Debug` stayed derived, so the `assert_eq!`s in `cases/deferred.rs`, the
only place a request is compared, still printed the arena on failure.
`Debug` forwards to `Display`.

`Nodes::linear` asked nothing of the arena beside it: it is `Operand`'s
question, the way `RequestedLen::linear` is `RequestedLen`'s.

`describe`'s `|r| format!("{r}")` shadowed the `r: &SizeRule` four lines
above it.

Format, workspace clippy under -D warnings with and without
layout-diagnostics, 207 ordinary and 211 diagnostic tests, the cold dump
byte-identical to 05e6ced across all 34,986 boxes, and 400 depth-5 trees
in each of the three deferred corpora in 200.95s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
iris-aiandClaude Opus 5 committed 2026-09-20 21:33:54 -04:00
1 parent 05e6ced31d
commit f48e04ed36
4 files changed
+45 -11

No files matched your search

+20 -10
View File
@@ -40,6 +40,16 @@ enum Operand {
Node(u32), Node(u32),
} }
impl Operand {
/// The length itself, where no comparison is waiting on an allocation.
fn linear(&self) -> Option<LayoutLen> {
match *self {
Self::Linear(len) => Some(len),
Self::Node(_) => None,
}
}
}
/// One sum or comparison, with its two operands. Whether anything under it /// One sum or comparison, with its two operands. Whether anything under it
/// divides leftover space is carried on the node rather than walked for, /// divides leftover space is carried on the node rather than walked for,
/// because every caller of one asks. /// because every caller of one asks.
@@ -63,14 +73,6 @@ impl Nodes {
self.0[index as usize] self.0[index as usize]
} }
/// The length itself, where no comparison is waiting on an allocation.
fn linear(&self, at: Operand) -> Option<LayoutLen> {
match at {
Operand::Linear(len) => Some(len),
Operand::Node(_) => None,
}
}
fn leftover(&self, at: Operand) -> bool { fn leftover(&self, at: Operand) -> bool {
match at { match at {
Operand::Linear(len) => len.leftover > Weight::ZERO, Operand::Linear(len) => len.leftover > Weight::ZERO,
@@ -82,7 +84,7 @@ impl Nodes {
/// lengths that keep their order whatever the room comes to are already /// lengths that keep their order whatever the room comes to are already
/// decided, and so are two operands that are the same thing. /// decided, and so are two operands that are the same thing.
fn combine(&mut self, op: Op, a: Operand, b: Operand) -> Operand { fn combine(&mut self, op: Op, a: Operand, b: Operand) -> Operand {
if let (Some(x), Some(y)) = (self.linear(a), self.linear(b)) { if let (Some(x), Some(y)) = (a.linear(), b.linear()) {
if matches!(op, Op::Sum) { if matches!(op, Op::Sum) {
return Operand::Linear(x + y); return Operand::Linear(x + y);
} }
@@ -150,7 +152,7 @@ impl Nodes {
/// An expression is the same nodes the layout pass allocates, in an arena of /// An expression is the same nodes the layout pass allocates, in an arena of
/// its own: importing one copies those nodes into the pass's arena, so there /// its own: importing one copies those nodes into the pass's arena, so there
/// is no second shape to keep in step and one place where folding is decided. /// is no second shape to keep in step and one place where folding is decided.
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, PartialEq)]
pub enum SizeRequest { pub enum SizeRequest {
Linear(LayoutLen), Linear(LayoutLen),
/// Behind a pointer, because a plain length is what nearly every rule /// Behind a pointer, because a plain length is what nearly every rule
@@ -237,6 +239,14 @@ impl std::fmt::Display for SizeRequest {
} }
} }
/// The same, since an arena printed as a struct is not a tree anyone can
/// write out again, which is what a request is printed for.
impl std::fmt::Debug for SizeRequest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self, f)
}
}
/// A discovered length. Deferred values belong to the current layout pass; /// A discovered length. Deferred values belong to the current layout pass;
/// widgets must not retain them. Ordinary requests remain inline lengths. /// widgets must not retain them. Ordinary requests remain inline lengths.
#[derive(Clone, Copy, Debug, PartialEq)] #[derive(Clone, Copy, Debug, PartialEq)]
+21
View File
@@ -132,6 +132,27 @@ fn a_deferred_comparison_can_compare_two_different_weights() {
assert_corners!(h, b, (200, 0), (300, 100)); assert_corners!(h, b, (200, 0), (300, 100));
} }
#[test]
fn a_comparison_between_two_comparisons_keeps_both_of_them() {
// Joining two expressions is the one path that copies a request's nodes
// into another's arena; the floor puts a node under the copied one, so
// its operands have to be renumbered as they land.
let capped = leftover(1).min(px(40));
let floored = leftover(2).min(px(70)).max(px(10));
let mut h = Harness::new((90, 100));
let both = rect(Color::RED).width(capped.max(floored)).add(&mut h.rsc);
let rest = rect(Color::BLUE).add(&mut h.rsc);
h.set_root((both, rest).span(Dir::RIGHT));
// Under either cap, so the doubled share is the longer of the two.
assert_corners!(h, both, (0, 0), (60, 100));
assert_corners!(h, rest, (60, 0), (90, 100));
h.resize((300, 100));
h.frame();
// Over both caps, so the comparison is between 40 and 70.
assert_corners!(h, both, (0, 0), (70, 100));
assert_corners!(h, rest, (70, 0), (300, 100));
}
#[test] #[test]
fn a_length_expression_is_resolved_before_wrapping_text() { fn a_length_expression_is_resolved_before_wrapping_text() {
let mut h = Harness::new((300, 500)); let mut h = Harness::new((300, 500));
+3
View File
@@ -38,6 +38,9 @@ fn deferred_requests_agree_warm_and_cold() {
0 => leftover(1).clamp(20, 120).into(), 0 => leftover(1).clamp(20, 120).into(),
1 => leftover(1).min(rel(0.5)).into(), 1 => leftover(1).min(rel(0.5)).into(),
2 => (leftover(1) + px(30)).min(leftover(2)).into(), 2 => (leftover(1) + px(30)).min(leftover(2)).into(),
// Both sides an expression, the one shape that
// copies a request's nodes into another's.
3 => leftover(1).min(px(40)).max(leftover(2).min(px(70))).into(),
_ => rules[axis].clone(), _ => rules[axis].clone(),
}; };
} }
+1 -1
View File
@@ -357,7 +357,7 @@ fn describe(id: WidgetId, h: &Harness) -> String {
let mut out = r let mut out = r
.request .request
.as_ref() .as_ref()
.map_or_else(String::new, |r| format!("{r}")); .map_or_else(String::new, ToString::to_string);
if let Some(min) = r.bound.min { if let Some(min) = r.bound.min {
out += &format!(">{}", LayoutLen::from(min)); out += &format!(">{}", LayoutLen::from(min));
} }