From 5965113fc8ac84b07b2c9cac4b4003efd7e6728a Mon Sep 17 00:00:00 2001 From: Julia Date: Mon, 28 Nov 2022 23:34:38 -0500 Subject: [PATCH 1/4] Add verify macros & use in one location for point conversion --- Cargo.lock | 9 +++++++++ Cargo.toml | 1 + crates/rope/Cargo.toml | 2 +- crates/rope/src/rope.rs | 10 ++++++---- crates/verify/Cargo.toml | 11 +++++++++++ crates/verify/src/verify.rs | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 crates/verify/Cargo.toml create mode 100644 crates/verify/src/verify.rs diff --git a/Cargo.lock b/Cargo.lock index 93631697c1..550b240b65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4778,6 +4778,7 @@ dependencies = [ "smallvec", "sum_tree", "util", + "verify", ] [[package]] @@ -6844,6 +6845,14 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" +[[package]] +name = "verify" +version = "0.1.0" +dependencies = [ + "backtrace", + "log", +] + [[package]] name = "version_check" version = "0.9.4" diff --git a/Cargo.toml b/Cargo.toml index 8e9814c448..1461855e22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ members = [ "crates/theme_selector", "crates/theme_testbench", "crates/util", + "crates/verify", "crates/vim", "crates/workspace", "crates/zed", diff --git a/crates/rope/Cargo.toml b/crates/rope/Cargo.toml index 0f754c1fb3..fb7836fab8 100644 --- a/crates/rope/Cargo.toml +++ b/crates/rope/Cargo.toml @@ -12,7 +12,7 @@ smallvec = { version = "1.6", features = ["union"] } sum_tree = { path = "../sum_tree" } arrayvec = "0.7.1" log = { version = "0.4.16", features = ["kv_unstable_serde"] } - +verify = { path = "../verify" } [dev-dependencies] rand = "0.8.3" diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index d4ee894310..03810be0b9 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -12,6 +12,7 @@ use std::{ str, }; use sum_tree::{Bias, Dimension, SumTree}; +use verify::{verify, verify_not}; pub use offset_utf16::OffsetUtf16; pub use point::Point; @@ -680,10 +681,11 @@ impl Chunk { let mut offset = 0; let mut point = Point::new(0, 0); for ch in self.0.chars() { - if point >= target { - if point > target { - panic!("point {:?} is inside of character {:?}", target, ch); - } + verify_not!(point > target, ("point {:?} is inside of character {:?}", target, ch), else { + point = target; + }); + + if point == target { break; } diff --git a/crates/verify/Cargo.toml b/crates/verify/Cargo.toml new file mode 100644 index 0000000000..72d64511f3 --- /dev/null +++ b/crates/verify/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "verify" +version = "0.1.0" +edition = "2021" + +[lib] +path = "src/verify.rs" + +[dependencies] +backtrace = "0.3" +log = "0.4" diff --git a/crates/verify/src/verify.rs b/crates/verify/src/verify.rs new file mode 100644 index 0000000000..9e1a4a5c89 --- /dev/null +++ b/crates/verify/src/verify.rs @@ -0,0 +1,33 @@ +pub use backtrace::Backtrace; + +#[macro_export] +macro_rules! verify { + ( $expression:expr, else $block:expr ) => { + verify!($expression, (""), else $block) + }; + + ( $expression:expr, ( $($fmt_arg:tt)* ), else $block:expr ) => {{ + let verify_str = stringify!($expression); + + if !$expression { + if cfg!(debug_assertions) { + panic!("Claim failed {:?}: {}", verify_str, format_args!($($fmt_arg)*)); + } else { + let backtrace = $crate::Backtrace::new(); + log::error!("Claim failed {:?}\n{:?}", verify_str, backtrace); + $block + } + } + }}; +} + +#[macro_export] +macro_rules! verify_not { + ( $expression:expr, else $block:expr ) => { + verify_not!($expression, (""), else $block) + }; + + ( $expression:expr, ( $($fmt_arg:tt)* ), else $block:expr ) => { + verify!(!$expression, ( $($fmt_arg)* ), else $block) + }; +} From 2b979d3b88f7025407c0ee0a65a9d90a96f02685 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 30 Nov 2022 00:01:40 -0500 Subject: [PATCH 2/4] Don't panic rope point conversions --- crates/rope/src/rope.rs | 47 ++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index 03810be0b9..569d48dcbd 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -681,8 +681,8 @@ impl Chunk { let mut offset = 0; let mut point = Point::new(0, 0); for ch in self.0.chars() { - verify_not!(point > target, ("point {:?} is inside of character {:?}", target, ch), else { - point = target; + verify_not!(point > target, ("point {target:?} is inside of character {ch:?}"), else { + return offset; }); if point == target { @@ -691,16 +691,19 @@ impl Chunk { if ch == '\n' { point.row += 1; - if point.row > target.row { - panic!( - "point {:?} is beyond the end of a line with length {}", - target, point.column - ); - } point.column = 0; + + verify_not!( + point.row > target.row, + ("point {target:?} is beyond the end of a line with length {}", point.column), + else { + return offset; + } + ); } else { point.column += ch.len_utf8() as u32; } + offset += ch.len_utf8(); } offset @@ -739,26 +742,36 @@ impl Chunk { if ch == '\n' { point.row += 1; point.column = 0; - if point.row > target.row { - if clip { + + if clip { + if point.row > target.row { // Return the offset of the newline return offset; } - panic!( - "point {:?} is beyond the end of a line with length {}", - target, point.column - ); + } else { + verify_not!( + point.row > target.row, + ("point {target:?} is beyond the end of a line with length {}", point.column), + else { + // Return the offset of the newline + return offset; + } + ) } } else { point.column += ch.len_utf16() as u32; } - if point > target { - if clip { + if clip { + if point > target { // Return the offset of the codepoint which we have landed within, bias left return offset; } - panic!("point {:?} is inside of codepoint {:?}", target, ch); + } else { + verify_not!(point > target, ("point {target:?} is inside of codepoint {ch:?}"), else { + // Return the offset of the codepoint which we have landed within, bias left + return offset; + }); } offset += ch.len_utf8(); From 023ecd595b7248c1a7f8b13a2307ed54692e1a5d Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 30 Nov 2022 13:03:15 -0500 Subject: [PATCH 3/4] Change verify macro to debug panic Co-Authored-By: Max Brunsfeld --- Cargo.lock | 10 +------ Cargo.toml | 1 - crates/rope/Cargo.toml | 2 +- crates/rope/src/rope.rs | 56 ++++++++++++++++--------------------- crates/util/Cargo.toml | 1 + crates/util/src/lib.rs | 13 +++++++++ crates/verify/Cargo.toml | 11 -------- crates/verify/src/verify.rs | 33 ---------------------- 8 files changed, 40 insertions(+), 87 deletions(-) delete mode 100644 crates/verify/Cargo.toml delete mode 100644 crates/verify/src/verify.rs diff --git a/Cargo.lock b/Cargo.lock index 550b240b65..24cd7a7748 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4778,7 +4778,6 @@ dependencies = [ "smallvec", "sum_tree", "util", - "verify", ] [[package]] @@ -6786,6 +6785,7 @@ name = "util" version = "0.1.0" dependencies = [ "anyhow", + "backtrace", "futures 0.3.24", "git2", "lazy_static", @@ -6845,14 +6845,6 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" -[[package]] -name = "verify" -version = "0.1.0" -dependencies = [ - "backtrace", - "log", -] - [[package]] name = "version_check" version = "0.9.4" diff --git a/Cargo.toml b/Cargo.toml index 1461855e22..8e9814c448 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,7 +52,6 @@ members = [ "crates/theme_selector", "crates/theme_testbench", "crates/util", - "crates/verify", "crates/vim", "crates/workspace", "crates/zed", diff --git a/crates/rope/Cargo.toml b/crates/rope/Cargo.toml index fb7836fab8..bd1dc690db 100644 --- a/crates/rope/Cargo.toml +++ b/crates/rope/Cargo.toml @@ -12,7 +12,7 @@ smallvec = { version = "1.6", features = ["union"] } sum_tree = { path = "../sum_tree" } arrayvec = "0.7.1" log = { version = "0.4.16", features = ["kv_unstable_serde"] } -verify = { path = "../verify" } +util = { path = "../util" } [dev-dependencies] rand = "0.8.3" diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index 569d48dcbd..e4f2bf5011 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -12,7 +12,7 @@ use std::{ str, }; use sum_tree::{Bias, Dimension, SumTree}; -use verify::{verify, verify_not}; +use util::debug_panic; pub use offset_utf16::OffsetUtf16; pub use point::Point; @@ -681,9 +681,10 @@ impl Chunk { let mut offset = 0; let mut point = Point::new(0, 0); for ch in self.0.chars() { - verify_not!(point > target, ("point {target:?} is inside of character {ch:?}"), else { + if point > target { + debug_panic!("point {target:?} is inside of character {ch:?}"); return offset; - }); + } if point == target { break; @@ -693,13 +694,13 @@ impl Chunk { point.row += 1; point.column = 0; - verify_not!( - point.row > target.row, - ("point {target:?} is beyond the end of a line with length {}", point.column), - else { - return offset; - } - ); + if point.row > target.row { + debug_panic!( + "point {target:?} is beyond the end of a line with length {}", + point.column + ); + return offset; + } } else { point.column += ch.len_utf8() as u32; } @@ -743,35 +744,26 @@ impl Chunk { point.row += 1; point.column = 0; - if clip { - if point.row > target.row { - // Return the offset of the newline - return offset; + if point.row > target.row { + if !clip { + debug_panic!( + "point {target:?} is beyond the end of a line with length {}", + point.column + ); } - } else { - verify_not!( - point.row > target.row, - ("point {target:?} is beyond the end of a line with length {}", point.column), - else { - // Return the offset of the newline - return offset; - } - ) + // Return the offset of the newline + return offset; } } else { point.column += ch.len_utf16() as u32; } - if clip { - if point > target { - // Return the offset of the codepoint which we have landed within, bias left - return offset; + if point > target { + if !clip { + debug_panic!("point {target:?} is inside of codepoint {ch:?}"); } - } else { - verify_not!(point > target, ("point {target:?} is inside of codepoint {ch:?}"), else { - // Return the offset of the codepoint which we have landed within, bias left - return offset; - }); + // Return the offset of the codepoint which we have landed within, bias left + return offset; } offset += ch.len_utf8(); diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index c083137156..fc16eeb53c 100644 --- a/crates/util/Cargo.toml +++ b/crates/util/Cargo.toml @@ -11,6 +11,7 @@ test-support = ["serde_json", "tempdir", "git2"] [dependencies] anyhow = "1.0.38" +backtrace = "0.3" futures = "0.3" log = { version = "0.4.16", features = ["kv_unstable_serde"] } lazy_static = "1.4.0" diff --git a/crates/util/src/lib.rs b/crates/util/src/lib.rs index e35f2df7d4..22d63a0996 100644 --- a/crates/util/src/lib.rs +++ b/crates/util/src/lib.rs @@ -1,6 +1,7 @@ #[cfg(any(test, feature = "test-support"))] pub mod test; +pub use backtrace::Backtrace; use futures::Future; use rand::{seq::SliceRandom, Rng}; use std::{ @@ -10,6 +11,18 @@ use std::{ task::{Context, Poll}, }; +#[macro_export] +macro_rules! debug_panic { + ( $($fmt_arg:tt)* ) => { + if cfg!(debug_assertions) { + panic!( $($fmt_arg)* ); + } else { + let backtrace = $crate::Backtrace::new(); + log::error!("{}\n{:?}", format_args!($($fmt_arg)*), backtrace); + } + }; +} + pub fn truncate(s: &str, max_chars: usize) -> &str { match s.char_indices().nth(max_chars) { None => s, diff --git a/crates/verify/Cargo.toml b/crates/verify/Cargo.toml deleted file mode 100644 index 72d64511f3..0000000000 --- a/crates/verify/Cargo.toml +++ /dev/null @@ -1,11 +0,0 @@ -[package] -name = "verify" -version = "0.1.0" -edition = "2021" - -[lib] -path = "src/verify.rs" - -[dependencies] -backtrace = "0.3" -log = "0.4" diff --git a/crates/verify/src/verify.rs b/crates/verify/src/verify.rs deleted file mode 100644 index 9e1a4a5c89..0000000000 --- a/crates/verify/src/verify.rs +++ /dev/null @@ -1,33 +0,0 @@ -pub use backtrace::Backtrace; - -#[macro_export] -macro_rules! verify { - ( $expression:expr, else $block:expr ) => { - verify!($expression, (""), else $block) - }; - - ( $expression:expr, ( $($fmt_arg:tt)* ), else $block:expr ) => {{ - let verify_str = stringify!($expression); - - if !$expression { - if cfg!(debug_assertions) { - panic!("Claim failed {:?}: {}", verify_str, format_args!($($fmt_arg)*)); - } else { - let backtrace = $crate::Backtrace::new(); - log::error!("Claim failed {:?}\n{:?}", verify_str, backtrace); - $block - } - } - }}; -} - -#[macro_export] -macro_rules! verify_not { - ( $expression:expr, else $block:expr ) => { - verify_not!($expression, (""), else $block) - }; - - ( $expression:expr, ( $($fmt_arg:tt)* ), else $block:expr ) => { - verify!(!$expression, ( $($fmt_arg)* ), else $block) - }; -} From 41b2fde10d8285d4e77c246fdcb330d3850d4f23 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 30 Nov 2022 13:11:08 -0500 Subject: [PATCH 4/4] Style Co-Authored-By: Max Brunsfeld --- crates/rope/src/rope.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index e4f2bf5011..53713e3f7a 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -680,13 +680,12 @@ impl Chunk { fn point_to_offset(&self, target: Point) -> usize { let mut offset = 0; let mut point = Point::new(0, 0); - for ch in self.0.chars() { - if point > target { - debug_panic!("point {target:?} is inside of character {ch:?}"); - return offset; - } - if point == target { + for ch in self.0.chars() { + if point >= target { + if point > target { + debug_panic!("point {target:?} is inside of character {ch:?}"); + } break; } @@ -699,7 +698,7 @@ impl Chunk { "point {target:?} is beyond the end of a line with length {}", point.column ); - return offset; + break; } } else { point.column += ch.len_utf8() as u32; @@ -707,6 +706,7 @@ impl Chunk { offset += ch.len_utf8(); } + offset }