Remove match & TryFrom boilerplate (#603)

* refactor: Refactor match statements for duplicate arms - Consolidated redundant match arms in several files to streamline the code - Introduced project-wide clippy lint `match_same_arms` in the `.cargo/config` file. * feat: Refactor enum conversion using `TryFromRepr` macro - Implemented use of `TryFromRepr` macro for enum type conversion instead of custom `TryFrom<u16>` in `tag.rs`. - Implemented `proc_macro_derive(TryFromRepr)` for automatic deriving of `TryFrom<foo>` for enum type T with `#[repr(foo)]`. * fix: revert & except change to HashName impls
argumentcomputer · Aug 13, 2023 · a1c0ad0 · a1c0ad0
1 parent d85f592
commit a1c0ad0
Show file tree

Hide file tree

Showing 13 changed files with 168 additions and 197 deletions.
diff --git a/.cargo/config b/.cargo/config
@@ -7,4 +7,5 @@ xclippy = [
     "clippy", "--workspace", "--all-targets", "--",
     "-Wclippy::all",
     "-Wclippy::disallowed_methods",
+    "-Wclippy::match_same_arms",
 ]
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lurk-macros/Cargo.toml b/lurk-macros/Cargo.toml
@@ -19,6 +19,7 @@ proptest-derive = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 
 [dev-dependencies]
+anyhow.workspace = true
 bincode = { workspace = true }
 lurk_crate = { path = "../", package = "lurk" }
 pasta_curves = { workspace = true, features = ["repr-c", "serde"] }
diff --git a/lurk-macros/src/lib.rs b/lurk-macros/src/lib.rs
@@ -19,7 +19,7 @@ use proc_macro2::Span;
 use quote::{quote, ToTokens};
 use syn::{
     parse_macro_input, AttributeArgs, Data, DataEnum, DeriveInput, Ident, Item, Lit, Meta,
-    MetaList, NestedMeta, Type,
+    MetaList, NestedMeta, Path, Type,
 };
 
 #[proc_macro_derive(Coproc)]
@@ -375,3 +375,102 @@ fn parse_type(m: &NestedMeta) -> Type {
         }
     }
 }
+
+fn try_from_match_arms(
+    name: &Ident,
+    variant_names: &[&Ident],
+    ty: syn::Path,
+) -> proc_macro2::TokenStream {
+    let mut match_arms = quote! {};
+    for variant in variant_names {
+        match_arms.extend(quote! {
+            x if x == #name::#variant as #ty => Ok(#name::#variant),
+        });
+    }
+    match_arms
+}
+
+fn get_type_from_attrs(attrs: &[syn::Attribute], attr_name: &str) -> syn::Result<Path> {
+    let Some(nested_arg) = attrs.iter().find_map(|arg| {
+        let Ok(Meta::List(MetaList { path, nested, .. })) = arg.parse_meta() else {
+            return None;
+        };
+        if !path.is_ident(attr_name) {
+            return None;
+        }
+        nested.first().cloned()
+    }) else {
+        return Err(syn::Error::new(
+            proc_macro2::Span::call_site(),
+            format!("Could not find attribute {}", attr_name),
+        ));
+    };
+
+    match nested_arg {
+        NestedMeta::Meta(Meta::Path(path)) => Ok(path),
+        bad => Err(syn::Error::new_spanned(
+            bad,
+            &format!("Could not parse {} attribute", attr_name)[..],
+        )),
+    }
+}
+
+/// This macro derives an impl of TryFrom<foo> for an enum type T with `#[repr(foo)]`.
+///
+/// # Example
+/// ```
+/// use lurk_macros::TryFromRepr;
+///
+/// #[derive(TryFromRepr)]
+/// #[repr(u8)]
+/// enum Foo {
+///     Bar = 0,
+///     Baz
+/// }
+/// ```
+///
+/// This will derive the natural impl that compares the input representation type to
+/// the automatic conversions of each variant into that representation type.
+#[proc_macro_derive(TryFromRepr)]
+pub fn derive_try_from_repr(input: TokenStream) -> TokenStream {
+    let ast = parse_macro_input!(input as DeriveInput);
+    let res_ty = get_type_from_attrs(&ast.attrs, "repr");
+
+    let name = &ast.ident;
+    let variants = match ast.data {
+        Data::Enum(ref variants) => variants
+            .variants
+            .iter()
+            .map(|v| &v.ident)
+            .collect::<Vec<_>>(),
+        Data::Struct(_) | Data::Union(_) => {
+            panic!("#[derive(TryFromRepr)] is only defined for enums")
+        }
+    };
+
+    match res_ty {
+        Err(e) => {
+            // If no explicit repr were given for us, we can't pursue
+            panic!(
+                "TryFromRepr macro requires a repr parameter, which couldn't be parsed: {:?}",
+                e
+            );
+        }
+        Ok(ty) => {
+            let match_arms = try_from_match_arms(name, &variants, ty.clone());
+            let name_str = name.to_string();
+            quote! {
+                impl std::convert::TryFrom<#ty> for #name {
+                    type Error = anyhow::Error;
+                    fn try_from(v: #ty) -> Result<Self, <Self as TryFrom<#ty>>::Error> {
+                        match v {
+                            #match_arms
+                            _ => Err(anyhow::anyhow!("invalid variant for enum {}", #name_str)),
+                        }
+                    }
+                }
+            }
+        }
+    }
+    .into()
+}
diff --git a/src/cli/repl.rs b/src/cli/repl.rs
@@ -533,11 +533,11 @@ impl Repl<F> {
                     .eval_expr(second)
                     .with_context(|| "evaluating second arg")?;
                 let Some(secret) = self.store.fetch_num(&first_io.expr) else {
-                        bail!(
-                            "Secret must be a number. Got {}",
-                            first_io.expr.fmt_to_string(&self.store)
-                        )
-                    };
+                    bail!(
+                        "Secret must be a number. Got {}",
+                        first_io.expr.fmt_to_string(&self.store)
+                    )
+                };
                 self.hide(secret.into_scalar(), second_io.expr)?;
             }
             "fetch" => {

diff --git a/src/eval/reduction.rs b/src/eval/reduction.rs
@@ -38,9 +38,9 @@ enum Control<F: LurkField> {
 impl<F: LurkField> Control<F> {
     fn into_results(self, store: &mut Store<F>) -> (Ptr<F>, Ptr<F>, ContPtr<F>) {
         match self {
-            Self::Return(expr, env, cont) => (expr, env, cont),
-            Self::MakeThunk(expr, env, cont) => (expr, env, cont),
-            Self::ApplyContinuation(expr, env, cont) => (expr, env, cont),
+            Self::Return(expr, env, cont)
+            | Self::MakeThunk(expr, env, cont)
+            | Self::ApplyContinuation(expr, env, cont) => (expr, env, cont),
             Self::Error(expr, env) => (expr, env, store.intern_cont_error()),
         }
     }

diff --git a/src/hash_witness.rs b/src/hash_witness.rs
@@ -82,6 +82,7 @@ pub trait HashName {
 
 impl HashName for ConsName {
     fn index(&self) -> usize {
+        #[allow(clippy::match_same_arms)]
         match self {
             Self::NeverUsed => MAX_CONSES_PER_REDUCTION + 1,
             Self::Expr => 0,
@@ -131,6 +132,7 @@ pub enum ContName {
 
 impl HashName for ContName {
     fn index(&self) -> usize {
+        #[allow(clippy::match_same_arms)]
         match self {
             Self::NeverUsed => MAX_CONTS_PER_REDUCTION + 1,
             Self::ApplyContinuation => 0,

diff --git a/src/lem/circuit.rs b/src/lem/circuit.rs
@@ -839,15 +839,7 @@ impl Func {
                     Op::Cast(_tgt, tag, _src) => {
                         globals.insert(FWrap(tag.to_field()));
                     }
-                    Op::Add(_, _, _) => {
-                        globals.insert(FWrap(Tag::Expr(Num).to_field()));
-                        num_constraints += 1;
-                    }
-                    Op::Sub(_, _, _) => {
-                        globals.insert(FWrap(Tag::Expr(Num).to_field()));
-                        num_constraints += 1;
-                    }
-                    Op::Mul(_, _, _) => {
+                    Op::Add(_, _, _) | Op::Sub(_, _, _) | Op::Mul(_, _, _) => {
                         globals.insert(FWrap(Tag::Expr(Num).to_field()));
                         num_constraints += 1;
                     }
@@ -877,11 +869,7 @@ impl Func {
                         // one constraint for the image's hash
                         num_constraints += 1;
                     }
-                    Op::Hide(..) => {
-                        // TODO
-                        globals.insert(FWrap(F::ZERO));
-                    }
-                    Op::Open(..) => {
+                    Op::Hide(..) | Op::Open(..) => {
                         // TODO
                         globals.insert(FWrap(F::ZERO));
                     }

diff --git a/src/lem/pointers.rs b/src/lem/pointers.rs
@@ -35,10 +35,7 @@ impl<F: LurkField> std::hash::Hash for Ptr<F> {
 impl<F: LurkField> Ptr<F> {
     pub fn tag(&self) -> &Tag {
         match self {
-            Ptr::Leaf(tag, _) => tag,
-            Ptr::Tree2(tag, _) => tag,
-            Ptr::Tree3(tag, _) => tag,
-            Ptr::Tree4(tag, _) => tag,
+            Ptr::Leaf(tag, _) | Ptr::Tree2(tag, _) | Ptr::Tree3(tag, _) | Ptr::Tree4(tag, _) => tag,
         }
     }
 

diff --git a/src/parser/syntax.rs b/src/parser/syntax.rs
@@ -348,17 +348,7 @@ pub mod tests {
     {
         match (expected, p.parse(Span::<'a>::new(i))) {
             (Some(expected), Ok((_, x))) if x == expected => true,
-            (Some(_), Ok(..)) => {
-                // println!("input: {:?}", i);
-                // println!("expected: {} {:?}", expected.clone(), expected);
-                // println!("detected: {} {:?}", x.clone(), x);
-                false
-            }
-            (Some(..), Err(_)) => {
-                // println!("{}", e);
-                false
-            }
-            (None, Ok(..)) => {
+            (Some(_), Ok(..)) | (Some(..), Err(_)) | (None, Ok(..)) => {
                 // println!("input: {:?}", i);
                 // println!("expected parse error");
                 // println!("detected: {:?}", x);

diff --git a/src/store.rs b/src/store.rs
@@ -663,13 +663,7 @@ impl<F: LurkField> Store<F> {
     // fetch a symbol cons or keyword cons
     pub fn fetch_symcons(&self, ptr: &Ptr<F>) -> Option<(Ptr<F>, Ptr<F>)> {
         match (ptr.tag, ptr.raw) {
-            (ExprTag::Sym, RawPtr::Null) => None,
-            (ExprTag::Key, RawPtr::Null) => None,
-            (ExprTag::Sym, RawPtr::Index(x)) => {
-                let (car, cdr) = self.sym_store.get_index(x)?;
-                Some((*car, *cdr))
-            }
-            (ExprTag::Key, RawPtr::Index(x)) => {
+            (ExprTag::Sym, RawPtr::Index(x)) | (ExprTag::Key, RawPtr::Index(x)) => {
                 let (car, cdr) = self.sym_store.get_index(x)?;
                 Some((*car, *cdr))
             }
@@ -711,7 +705,6 @@ impl<F: LurkField> Store<F> {
 
     pub fn fetch_strcons(&self, ptr: &Ptr<F>) -> Option<(Ptr<F>, Ptr<F>)> {
         match (ptr.tag, ptr.raw) {
-            (ExprTag::Str, RawPtr::Null) => None,
             (ExprTag::Str, RawPtr::Index(x)) => {
                 let (car, cdr) = self.str_store.get_index(x)?;
                 Some((*car, *cdr))