From 2083eae2c5b480ccad672fb2fbb1b2bb5774a606 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Tue, 14 Dec 2021 03:49:13 -0500 Subject: replace all uses of unwrap(), expect(), and indexing with unreachable!() and also document why they are unreachable --- src/screen.rs | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 160 insertions(+), 25 deletions(-) (limited to 'src/screen.rs') diff --git a/src/screen.rs b/src/screen.rs index 9e81ee2..95951d9 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -292,7 +292,9 @@ impl Screen { ) -> impl Iterator> + '_ { let mut wrapping = false; self.grid().visible_rows().enumerate().map(move |(i, row)| { - let i = i.try_into().unwrap(); + // number of rows in a grid is stored in a u16 (see Size), so + // visible_rows can never return enough rows to overflow here + let i = i.try_into().unwrap_or_else(|_| unreachable!()); let mut contents = vec![]; row.write_contents_formatted( &mut contents, @@ -359,7 +361,9 @@ impl Screen { .zip(prev.grid().visible_rows()) .enumerate() .map(move |(i, (row, prev_row))| { - let i = i.try_into().unwrap(); + // number of rows in a grid is stored in a u16 (see Size), so + // visible_rows can never return enough rows to overflow here + let i = i.try_into().unwrap_or_else(|_| unreachable!()); let mut contents = vec![]; row.write_contents_diff( &mut contents, @@ -811,7 +815,11 @@ impl Screen { // don't even try to draw control characters return; } - let width = width.unwrap_or(1).try_into().unwrap(); + let width = width + .unwrap_or(1) + .try_into() + // width() can only return 0, 1, or 2 + .unwrap_or_else(|_| unreachable!()); // it doesn't make any sense to wrap if the last column in a row // didn't already have contents. don't try to handle the case where a @@ -829,7 +837,10 @@ impl Screen { row: pos.row, col: size.cols - 1, }) - .unwrap(); + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always have a valid + // row value. size.cols - 1 is also always a valid column. + .unwrap_or_else(|| unreachable!()); if last_cell.has_contents() || last_cell.is_wide_continuation() { wrap = true; } @@ -845,7 +856,11 @@ impl Screen { row: pos.row, col: pos.col - 1, }) - .unwrap(); + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always have a + // valid row value. pos.col - 1 is valid because we just + // checked for pos.col > 0. + .unwrap_or_else(|| unreachable!()); if prev_cell.is_wide_continuation() { prev_cell = self .grid_mut() @@ -853,11 +868,24 @@ impl Screen { row: pos.row, col: pos.col - 2, }) - .unwrap(); + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always have a + // valid row value. we know pos.col - 2 is valid + // because the cell at pos.col - 1 is a wide + // continuation character, which means there must be + // the first half of the wide character before it. + .unwrap_or_else(|| unreachable!()); } prev_cell.append(c); } else if pos.row > 0 { - let prev_row = self.grid().drawing_row(pos.row - 1).unwrap(); + let prev_row = self + .grid() + .drawing_row(pos.row - 1) + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always have a + // valid row value. pos.row - 1 is valid because we just + // checked for pos.row > 0. + .unwrap_or_else(|| unreachable!()); if prev_row.wrapped() { let mut prev_cell = self .grid_mut() @@ -865,7 +893,12 @@ impl Screen { row: pos.row - 1, col: size.cols - 1, }) - .unwrap(); + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always have a + // valid row value. pos.row - 1 is valid because we + // just checked for pos.row > 0. col of size.cols - 1 + // is always valid. + .unwrap_or_else(|| unreachable!()); if prev_cell.is_wide_continuation() { prev_cell = self .grid_mut() @@ -873,7 +906,15 @@ impl Screen { row: pos.row - 1, col: size.cols - 2, }) - .unwrap(); + // pos.row is valid, since it comes directly from + // self.grid().pos() which we assume to always + // have a valid row value. pos.row - 1 is valid + // because we just checked for pos.row > 0. col of + // size.cols - 2 is valid because the cell at + // size.cols - 1 is a wide continuation character, + // so it must have the first half of the wide + // character before it. + .unwrap_or_else(|| unreachable!()); } prev_cell.append(c); } @@ -882,7 +923,11 @@ impl Screen { if self .grid() .drawing_cell(pos) - .unwrap() + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because we + // called col_wrap() immediately before this, which ensures + // that self.grid().pos().col has a valid value. + .unwrap_or_else(|| unreachable!()) .is_wide_continuation() { let prev_cell = self @@ -891,27 +936,69 @@ impl Screen { row: pos.row, col: pos.col - 1, }) - .unwrap(); + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because + // we called col_wrap() immediately before this, which + // ensures that self.grid().pos().col has a valid value. + // pos.col - 1 is valid because the cell at pos.col is a + // wide continuation character, so it must have the first + // half of the wide character before it. + .unwrap_or_else(|| unreachable!()); prev_cell.clear(attrs); } - if self.grid().drawing_cell(pos).unwrap().is_wide() { + if self + .grid() + .drawing_cell(pos) + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because we + // called col_wrap() immediately before this, which ensures + // that self.grid().pos().col has a valid value. + .unwrap_or_else(|| unreachable!()) + .is_wide() + { let next_cell = self .grid_mut() .drawing_cell_mut(crate::grid::Pos { row: pos.row, col: pos.col + 1, }) - .unwrap(); + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because + // we called col_wrap() immediately before this, which + // ensures that self.grid().pos().col has a valid value. + // pos.col + 1 is valid because the cell at pos.col is a + // wide character, so it must have the second half of the + // wide character after it. + .unwrap_or_else(|| unreachable!()); next_cell.set(' ', attrs); } - let cell = self.grid_mut().drawing_cell_mut(pos).unwrap(); + let cell = self + .grid_mut() + .drawing_cell_mut(pos) + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because we + // called col_wrap() immediately before this, which ensures + // that self.grid().pos().col has a valid value. + .unwrap_or_else(|| unreachable!()); cell.set(c, attrs); self.grid_mut().col_inc(1); if width > 1 { let pos = self.grid().pos(); - if self.grid().drawing_cell(pos).unwrap().is_wide() { + if self + .grid() + .drawing_cell(pos) + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because + // we called col_wrap() earlier, which ensures that + // self.grid().pos().col has a valid value. this is true + // even though we just called col_inc, because this branch + // only happens if width > 1, and col_wrap takes width + // into account. + .unwrap_or_else(|| unreachable!()) + .is_wide() + { let next_next_pos = crate::grid::Pos { row: pos.row, col: pos.col + 1, @@ -919,17 +1006,37 @@ impl Screen { let next_next_cell = self .grid_mut() .drawing_cell_mut(next_next_pos) - .unwrap(); + // pos.row is valid because we assume + // self.grid().pos() to always have a valid row value. + // pos.col is valid because we called col_wrap() + // earlier, which ensures that self.grid().pos().col + // has a valid value. this is true even though we just + // called col_inc, because this branch only happens if + // width > 1, and col_wrap takes width into account. + // pos.col + 1 is valid because the cell at pos.col is + // wide, and so it must have the second half of the + // wide character after it. + .unwrap_or_else(|| unreachable!()); next_next_cell.clear(attrs); if next_next_pos.col == size.cols - 1 { self.grid_mut() .drawing_row_mut(pos.row) - .unwrap() + // we assume self.grid().pos().row is always valid + .unwrap_or_else(|| unreachable!()) .wrap(false); } } - let next_cell = - self.grid_mut().drawing_cell_mut(pos).unwrap(); + let next_cell = self + .grid_mut() + .drawing_cell_mut(pos) + // pos.row is valid because we assume self.grid().pos() to + // always have a valid row value. pos.col is valid because + // we called col_wrap() earlier, which ensures that + // self.grid().pos().col has a valid value. this is true + // even though we just called col_inc, because this branch + // only happens if width > 1, and col_wrap takes width + // into account. + .unwrap_or_else(|| unreachable!()); next_cell.clear(crate::attrs::Attrs::default()); next_cell.set_wide_continuation(true); self.grid_mut().col_inc(1); @@ -1168,7 +1275,12 @@ impl Screen { ns => { if log::log_enabled!(log::Level::Debug) { let n = if ns.len() == 1 { - format!("{}", ns[0]) + format!( + "{}", + // we just checked that ns.len() == 1, so 0 + // must be valid + ns.get(0).unwrap_or_else(|| unreachable!()) + ) } else { format!("{:?}", ns) }; @@ -1222,7 +1334,12 @@ impl Screen { ns => { if log::log_enabled!(log::Level::Debug) { let n = if ns.len() == 1 { - format!("{}", ns[0]) + format!( + "{}", + // we just checked that ns.len() == 1, so 0 + // must be valid + ns.get(0).unwrap_or_else(|| unreachable!()) + ) } else { format!("{:?}", ns) }; @@ -1314,7 +1431,13 @@ impl Screen { ns => { if log::log_enabled!(log::Level::Debug) { let n = if ns.len() == 1 { - format!("{}", ns[0]) + format!( + "{}", + // we just checked that ns.len() == 1, so + // 0 must be valid + ns.get(0) + .unwrap_or_else(|| unreachable!()) + ) } else { format!("{:?}", ns) }; @@ -1355,7 +1478,13 @@ impl Screen { ns => { if log::log_enabled!(log::Level::Debug) { let n = if ns.len() == 1 { - format!("{}", ns[0]) + format!( + "{}", + // we just checked that ns.len() == 1, so + // 0 must be valid + ns.get(0) + .unwrap_or_else(|| unreachable!()) + ) } else { format!("{:?}", ns) }; @@ -1378,7 +1507,12 @@ impl Screen { ns => { if log::log_enabled!(log::Level::Debug) { let n = if ns.len() == 1 { - format!("{}", ns[0]) + format!( + "{}", + // we just checked that ns.len() == 1, so 0 + // must be valid + ns.get(0).unwrap_or_else(|| unreachable!()) + ) } else { format!("{:?}", ns) }; @@ -1614,7 +1748,8 @@ fn u16_to_u8(i: u16) -> Option { if i > u16::from(u8::max_value()) { None } else { - Some(i.try_into().unwrap()) + // safe because we just ensured that the value fits in a u8 + Some(i.try_into().unwrap_or_else(|_| unreachable!())) } } -- cgit v1.2.3-54-g00ecf