From afa34a24d8209b1f1a92e453e37892c98ca2df42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= <4142+huitseeker@users.noreply.github.com> Date: Sun, 18 Feb 2024 20:00:24 -0500 Subject: [PATCH] Use let-else syntax more (#1148) * refactor: avoid distant else, convert to `and_then` - Simplified return flow in `InversePoseidonCache` `get` method in `hash.rs` using `and_then`. - Refactored variable allocation within `from_ptr` method in `memoset/env.rs`. - Made minor clean-up and formatting improvements to improve readability across multiple files. * refactor: convert divergent if .. let to let .. else * refactor: Refactor if let .. else into let .. else where relevant in store.rs - Simplify code logic by replacing `if let` syntax with `let ... else`. * refactor: transform match on option to let-else where relevant --- lurk-metrics/src/lib.rs | 11 +- src/coroutine/memoset/env.rs | 24 +-- src/hash.rs | 8 +- src/lem/multiframe.rs | 14 +- src/lem/store.rs | 397 +++++++++++++++++------------------ src/lem/tag.rs | 9 +- 6 files changed, 214 insertions(+), 249 deletions(-) diff --git a/lurk-metrics/src/lib.rs b/lurk-metrics/src/lib.rs index d3b1d17023..a36bcaaa63 100644 --- a/lurk-metrics/src/lib.rs +++ b/lurk-metrics/src/lib.rs @@ -158,13 +158,12 @@ impl ThreadMetricsSinkHandle { /// Initialize the thread-local metrics sink by registering it with the global sink fn init() -> ThreadMetricsSinkHandle { - if let Some(global_sink) = GLOBAL_SINK.get() { - let me = Arc::new(Mutex::new(ThreadMetricsSink::default())); - global_sink.threads.lock().unwrap().push(Arc::clone(&me)); - ThreadMetricsSinkHandle { inner: me } - } else { + let Some(global_sink) = GLOBAL_SINK.get() else { panic!("global metrics sink must be installed first"); - } + }; + let me = Arc::new(Mutex::new(ThreadMetricsSink::default())); + global_sink.threads.lock().unwrap().push(Arc::clone(&me)); + ThreadMetricsSinkHandle { inner: me } } } diff --git a/src/coroutine/memoset/env.rs b/src/coroutine/memoset/env.rs index ef535b6392..feaf65293d 100644 --- a/src/coroutine/memoset/env.rs +++ b/src/coroutine/memoset/env.rs @@ -207,22 +207,16 @@ impl CircuitQuery for EnvCircuitQuery { fn from_ptr>(cs: &mut CS, s: &Store, ptr: &Ptr) -> Option { let query = EnvQuery::from_ptr(s, ptr); - if let Some(q) = query { - match q { - EnvQuery::Lookup(var, env) => { - let allocated_var = AllocatedNum::alloc_infallible(ns!(cs, "var"), || { - *s.hash_ptr(&var).value() - }); - let allocated_env = AllocatedNum::alloc_infallible(ns!(cs, "env"), || { - *s.hash_ptr(&env).value() - }); - Some(Self::Lookup(allocated_var, allocated_env)) - } - _ => unreachable!(), + query.and_then(|q| match q { + EnvQuery::Lookup(var, env) => { + let allocated_var = + AllocatedNum::alloc_infallible(ns!(cs, "var"), || *s.hash_ptr(&var).value()); + let allocated_env = + AllocatedNum::alloc_infallible(ns!(cs, "env"), || *s.hash_ptr(&env).value()); + Some(Self::Lookup(allocated_var, allocated_env)) } - } else { - None - } + _ => unreachable!(), + }) } fn dummy_from_index>(cs: &mut CS, s: &Store, index: usize) -> Self { diff --git a/src/hash.rs b/src/hash.rs index 24a1941442..c5706222c6 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -128,14 +128,10 @@ impl InversePoseidonCache { macro_rules! get { ($name:ident, $n: expr) => {{ let preimage = self.$name.get(key); - if let Some(p) = preimage { - assert_eq!(ARITY, $n); + preimage.and_then(|p | { assert_eq!(ARITY, $n); // SAFETY: we are just teaching the compiler that the slice has size, ARITY, which is guaranteed by // the assertion above. - Some(unsafe { std::mem::transmute::<&[F; $n], &[F; ARITY]>(p) }) - } else { - None - } + Some(unsafe { std::mem::transmute::<&[F; $n], &[F; ARITY]>(p) }) }) }}; } diff --git a/src/lem/multiframe.rs b/src/lem/multiframe.rs index 2c587cb354..8a49e09e1f 100644 --- a/src/lem/multiframe.rs +++ b/src/lem/multiframe.rs @@ -304,14 +304,12 @@ impl<'a, F: LurkField, C: Coprocessor> MultiFrame<'a, F, C> { inner_frames.push(frames[current_frame_idx].clone()); chunk_start_idx = current_frame_idx + 1; - if let Some(next_frame) = frames.get(chunk_start_idx) { - next_pc = next_frame.pc; - if next_pc != 0 { - // incompatible `pc` incoming - break; - } - } else { - // not enough frames + let Some(next_frame) = frames.get(chunk_start_idx) else { + break; + }; + next_pc = next_frame.pc; + if next_pc != 0 { + // incompatible `pc` incoming break; } } diff --git a/src/lem/store.rs b/src/lem/store.rs index 09110e7317..e4fd850b0d 100644 --- a/src/lem/store.rs +++ b/src/lem/store.rs @@ -865,21 +865,20 @@ impl Store { } let idx = ptr.raw().get_hash4()?; - if let Some([query_pay, val_tag, val_pay, deps_pay]) = self.fetch_raw_ptrs(idx) { - let query = Ptr::new(Tag::Expr(Cons), *query_pay); - let val = self.raw_to_ptr(val_tag, val_pay)?; + self.fetch_raw_ptrs(idx) + .and_then(|[query_pay, val_tag, val_pay, deps_pay]| { + let query = Ptr::new(Tag::Expr(Cons), *query_pay); + let val = self.raw_to_ptr(val_tag, val_pay)?; - let nil = self.intern_nil(); - let deps = if deps_pay == nil.raw() { - nil - } else { - Ptr::new(Tag::Expr(Prov), *deps_pay) - }; + let nil = self.intern_nil(); + let deps = if deps_pay == nil.raw() { + nil + } else { + Ptr::new(Tag::Expr(Prov), *deps_pay) + }; - Some((query, val, deps)) - } else { - None - } + Some((query, val, deps)) + }) } pub fn intern_syntax(&self, syn: Syntax) -> Ptr { @@ -1117,32 +1116,28 @@ impl Ptr { match self.tag() { Tag::Expr(t) => match t { Nil => { - if let Some(sym) = store.fetch_symbol(self) { - state.fmt_to_string(&sym.into()) - } else { - "".into() - } + let Some(sym) = store.fetch_symbol(self) else { + return "".into(); + }; + state.fmt_to_string(&sym.into()) } Sym => { - if let Some(sym) = store.fetch_sym(self) { - state.fmt_to_string(&sym.into()) - } else { - "".into() - } + let Some(sym) = store.fetch_sym(self) else { + return "".into(); + }; + state.fmt_to_string(&sym.into()) } Key => { - if let Some(key) = store.fetch_key(self) { - state.fmt_to_string(&key.into()) - } else { - "".into() - } + let Some(key) = store.fetch_key(self) else { + return "".into(); + }; + state.fmt_to_string(&key.into()) } Str => { - if let Some(str) = store.fetch_string(self) { - format!("\"{str}\"") - } else { - "".into() - } + let Some(str) = store.fetch_string(self) else { + return "".into(); + }; + format!("\"{str}\"") } Char => { if let Some(c) = self @@ -1157,34 +1152,30 @@ impl Ptr { } } Cons => { - if let Some((list, non_nil)) = store.fetch_list(self) { - let list = list - .iter() - .map(|p| p.fmt_to_string(store, state)) - .collect::>(); - if let Some(non_nil) = non_nil { - format!( - "({} . {})", - list.join(" "), - non_nil.fmt_to_string(store, state) - ) - } else { - format!("({})", list.join(" ")) - } - } else { - "".into() - } + let Some((list, non_nil)) = store.fetch_list(self) else { + return "".into(); + }; + let list = list + .iter() + .map(|p| p.fmt_to_string(store, state)) + .collect::>(); + let Some(non_nil) = non_nil else { + return format!("({})", list.join(" ")); + }; + format!( + "({} . {})", + list.join(" "), + non_nil.fmt_to_string(store, state) + ) } Num => { - if let Some(f) = self.raw().get_atom().map(|idx| store.expect_f(idx)) { - if let Some(u) = f.to_u64() { - u.to_string() - } else { - format!("0x{}", f.hex_digits()) - } - } else { - "".into() - } + let Some(f) = self.raw().get_atom().map(|idx| store.expect_f(idx)) else { + return "".into(); + }; + let Some(u) = f.to_u64() else { + return format!("0x{}", f.hex_digits()); + }; + u.to_string() } U64 => { if let Some(u) = self @@ -1198,128 +1189,119 @@ impl Ptr { "".into() } } - Fun => match self.raw().get_hash8() { - None => "".into(), - Some(idx) => { - if let Some([vars, body, _, _]) = fetch_ptrs!(store, 4, idx) { - match vars.tag() { - Tag::Expr(Nil) => { - format!("", body.fmt_to_string(store, state)) - } - Tag::Expr(Cons) => { - format!( - "", - vars.fmt_to_string(store, state), - body.fmt_to_string(store, state) - ) - } - _ => "".into(), - } - } else { - "".into() - } - } - }, - Rec => match self.raw().get_hash8() { - None => "".into(), - Some(idx) => { - if let Some([vars, body, _, _]) = fetch_ptrs!(store, 4, idx) { - match vars.tag() { - Tag::Expr(Nil) => { - format!( - "", - body.fmt_to_string(store, state) - ) - } - Tag::Expr(Cons) => { - format!( - "", - vars.fmt_to_string(store, state), - body.fmt_to_string(store, state) - ) - } - _ => "".into(), - } - } else { - "".into() + Fun => { + let Some(idx) = self.raw().get_hash8() else { + return "".into(); + }; + let Some([vars, body, _, _]) = fetch_ptrs!(store, 4, idx) else { + return "".into(); + }; + match vars.tag() { + Tag::Expr(Nil) => { + format!("", body.fmt_to_string(store, state)) } - } - }, - Thunk => match self.raw().get_hash4() { - None => "".into(), - Some(idx) => { - if let Some([val, cont]) = fetch_ptrs!(store, 2, idx) { + Tag::Expr(Cons) => { format!( - "Thunk{{ value: {} => cont: {} }}", - val.fmt_to_string(store, state), - cont.fmt_to_string(store, state) + "", + vars.fmt_to_string(store, state), + body.fmt_to_string(store, state) ) - } else { - "".into() } + _ => "".into(), } - }, - Comm => match self.raw().get_atom() { - Some(idx) => { - let f = store.expect_f(idx); - if store.comms.get(&FWrap(*f)).is_some() { - format!("(comm 0x{})", f.hex_digits()) - } else { - format!("", f.hex_digits()) + } + Rec => { + let Some(idx) = self.raw().get_hash8() else { + return "".into(); + }; + let Some([vars, body, _, _]) = fetch_ptrs!(store, 4, idx) else { + return "".into(); + }; + match vars.tag() { + Tag::Expr(Nil) => { + format!("", body.fmt_to_string(store, state)) } - } - None => "".into(), - }, - Cproc => match self.raw().get_hash4() { - None => "".into(), - Some(idx) => { - if let Some([cproc_name, args]) = fetch_ptrs!(store, 2, idx) { + Tag::Expr(Cons) => { format!( - "", - cproc_name.fmt_to_string(store, state), - args.fmt_to_string(store, state) + "", + vars.fmt_to_string(store, state), + body.fmt_to_string(store, state) ) - } else { - "".into() } + _ => "".into(), } - }, - Env => { - if let Some(env) = store.fetch_env(self) { - let list = env - .iter() - .map(|(sym, val)| { - format!( - "({} . {})", - sym.fmt_to_string(store, state), - val.fmt_to_string(store, state) - ) - }) - .collect::>(); - format!("", list.join(" ")) + } + Thunk => { + let Some(idx) = self.raw().get_hash4() else { + return "".into(); + }; + let Some([val, cont]) = fetch_ptrs!(store, 2, idx) else { + return "".into(); + }; + format!( + "Thunk{{ value: {} => cont: {} }}", + val.fmt_to_string(store, state), + cont.fmt_to_string(store, state) + ) + } + Comm => { + let Some(idx) = self.raw().get_atom() else { + return "".into(); + }; + let f = store.expect_f(idx); + if store.comms.get(&FWrap(*f)).is_some() { + format!("(comm 0x{})", f.hex_digits()) } else { - "".into() + format!("", f.hex_digits()) } } - Prov => { - if let Some((query, val, deps)) = store.fetch_provenance(self) { - let nil = store.intern_nil(); - if store.ptr_eq(&deps, &nil) { - format!( - "", - query.fmt_to_string(store, state), - val.fmt_to_string(store, state), - ) - } else { + Cproc => { + let Some(idx) = self.raw().get_hash4() else { + return "".into(); + }; + let Some([cproc_name, args]) = fetch_ptrs!(store, 2, idx) else { + return "".into(); + }; + format!( + "", + cproc_name.fmt_to_string(store, state), + args.fmt_to_string(store, state) + ) + } + Env => { + let Some(env) = store.fetch_env(self) else { + return "".into(); + }; + let list = env + .iter() + .map(|(sym, val)| { format!( - "", - query.fmt_to_string(store, state), - val.fmt_to_string(store, state), - deps.fmt_to_string(store, state) + "({} . {})", + sym.fmt_to_string(store, state), + val.fmt_to_string(store, state) ) - } + }) + .collect::>(); + format!("", list.join(" ")) + } + Prov => { + let Some((query, val, deps)) = store.fetch_provenance(self) else { + return "".into(); + }; + let nil = store.intern_nil(); + if store.ptr_eq(&deps, &nil) { + format!( + "", + query.fmt_to_string(store, state), + val.fmt_to_string(store, state), + ) } else { - "".into() + format!( + "", + query.fmt_to_string(store, state), + val.fmt_to_string(store, state), + deps.fmt_to_string(store, state) + ) } } }, @@ -1374,19 +1356,18 @@ impl Ptr { store: &Store, state: &State, ) -> String { - match self.raw().get_hash8() { - None => format!(""), - Some(idx) => { - if let Some([a, cont, _, _]) = fetch_ptrs!(store, 4, idx) { - format!( - "{name}{{ {field}: {}, continuation: {} }}", - a.fmt_to_string(store, state), - cont.fmt_to_string(store, state) - ) - } else { - format!("") - } - } + { + let Some(idx) = self.raw().get_hash8() else { + return format!(""); + }; + let Some([a, cont, _, _]) = fetch_ptrs!(store, 4, idx) else { + return format!(""); + }; + format!( + "{name}{{ {field}: {}, continuation: {} }}", + a.fmt_to_string(store, state), + cont.fmt_to_string(store, state) + ) } } @@ -1397,21 +1378,20 @@ impl Ptr { store: &Store, state: &State, ) -> String { - match self.raw().get_hash8() { - None => format!(""), - Some(idx) => { - if let Some([a, b, cont, _]) = fetch_ptrs!(store, 4, idx) { - let (fa, fb) = fields; - format!( - "{name}{{ {fa}: {}, {fb}: {}, continuation: {} }}", - a.fmt_to_string(store, state), - b.fmt_to_string(store, state), - cont.fmt_to_string(store, state) - ) - } else { - format!("") - } - } + { + let Some(idx) = self.raw().get_hash8() else { + return format!(""); + }; + let Some([a, b, cont, _]) = fetch_ptrs!(store, 4, idx) else { + return format!(""); + }; + let (fa, fb) = fields; + format!( + "{name}{{ {fa}: {}, {fb}: {}, continuation: {} }}", + a.fmt_to_string(store, state), + b.fmt_to_string(store, state), + cont.fmt_to_string(store, state) + ) } } @@ -1422,22 +1402,21 @@ impl Ptr { store: &Store, state: &State, ) -> String { - match self.raw().get_hash8() { - None => format!(""), - Some(idx) => { - if let Some([a, b, c, cont]) = fetch_ptrs!(store, 4, idx) { - let (fa, fb, fc) = fields; - format!( - "{name}{{ {fa}: {}, {fb}: {}, {fc}: {}, continuation: {} }}", - a.fmt_to_string(store, state), - b.fmt_to_string(store, state), - c.fmt_to_string(store, state), - cont.fmt_to_string(store, state) - ) - } else { - format!("") - } - } + { + let Some(idx) = self.raw().get_hash8() else { + return format!(""); + }; + let Some([a, b, c, cont]) = fetch_ptrs!(store, 4, idx) else { + return format!(""); + }; + let (fa, fb, fc) = fields; + format!( + "{name}{{ {fa}: {}, {fb}: {}, {fc}: {}, continuation: {} }}", + a.fmt_to_string(store, state), + b.fmt_to_string(store, state), + c.fmt_to_string(store, state), + cont.fmt_to_string(store, state) + ) } } } diff --git a/src/lem/tag.rs b/src/lem/tag.rs index 673e0d6d41..80ab7bf4ff 100644 --- a/src/lem/tag.rs +++ b/src/lem/tag.rs @@ -131,12 +131,11 @@ pub(crate) mod tests { #[test] fn pos_index_roundtrip() { for i in 0.. { - if let Some(tag) = Tag::pos(i) { - let j = tag.index(); - assert_eq!(i, j); - } else { + let Some(tag) = Tag::pos(i) else { break; - } + }; + let j = tag.index(); + assert_eq!(i, j); } for expr_tag in ExprTag::iter() {