reduce binary bloat by removing generic param from type_check

benma · benma · commit 0c411db3fbf9 · 2023-07-30T14:16:54.000+02:00
The binary sizes are increased a lot by the use of generics. In one
case, there is a lot of needless bloat by using a generic type param
for the callback argument in the type_check function of the Property
trait.

    cargo build --example parse --release &amp;&amp; du -sb target/release/examples/parse

Results in a binary size of 1784152 bytes.

With this commit, the binary size is decreased by 12288 bytes, which
is significant for embedded/no_std use.

The sizes above were achieved by adding this to Cargo.toml:

```
[profile.release]
strip = true
panic = "abort"
opt-level = 's'
codegen-units = 1
lto = true
```

Though the difference is roughly the same also without the
optimizations.

As an additional benefit, code clarity is improved as the child arg
was unused in some of the trait implementations.
diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs
@@ -47,7 +47,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Debug for Terminal<Pk, Ctx> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.write_str("[")?;
-        if let Ok(type_map) = types::Type::type_check(self, |_| None) {
+        if let Ok(type_map) = types::Type::type_check(self) {
             f.write_str(match type_map.corr.base {
                 types::Base::B => "B",
                 types::Base::K => "K",
diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs
@@ -25,10 +25,6 @@ use crate::prelude::*;
 use crate::Descriptor;
 use crate::{bitcoin, hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, ToPublicKey};
 
-fn return_none<T>(_: usize) -> Option<T> {
-    None
-}
-
 /// Trait for parsing keys from byte slices
 pub trait ParseableKey: Sized + ToPublicKey + private::Sealed {
     /// Parse a key from slice
@@ -223,8 +219,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
 
     ///reduce, type check and push a 0-arg node
     fn reduce0(&mut self, ms: Terminal<Pk, Ctx>) -> Result<(), Error> {
-        let ty = Type::type_check(&ms, return_none)?;
-        let ext = ExtData::type_check(&ms, return_none)?;
+        let ty = Type::type_check(&ms)?;
+        let ext = ExtData::type_check(&ms)?;
         let ms = Miniscript {
             node: ms,
             ty,
@@ -244,8 +240,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
         let top = self.pop().unwrap();
         let wrapped_ms = wrap(Arc::new(top));
 
-        let ty = Type::type_check(&wrapped_ms, return_none)?;
-        let ext = ExtData::type_check(&wrapped_ms, return_none)?;
+        let ty = Type::type_check(&wrapped_ms)?;
+        let ext = ExtData::type_check(&wrapped_ms)?;
         let ms = Miniscript {
             node: wrapped_ms,
             ty,
@@ -267,8 +263,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
 
         let wrapped_ms = wrap(Arc::new(left), Arc::new(right));
 
-        let ty = Type::type_check(&wrapped_ms, return_none)?;
-        let ext = ExtData::type_check(&wrapped_ms, return_none)?;
+        let ty = Type::type_check(&wrapped_ms)?;
+        let ext = ExtData::type_check(&wrapped_ms)?;
         let ms = Miniscript {
             node: wrapped_ms,
             ty,
@@ -556,8 +552,8 @@ pub fn parse<Ctx: ScriptContext>(
                 let c = term.pop().unwrap();
                 let wrapped_ms = Terminal::AndOr(Arc::new(a), Arc::new(c), Arc::new(b));
 
-                let ty = Type::type_check(&wrapped_ms, return_none)?;
-                let ext = ExtData::type_check(&wrapped_ms, return_none)?;
+                let ty = Type::type_check(&wrapped_ms)?;
+                let ext = ExtData::type_check(&wrapped_ms)?;
 
                 term.0.push(Miniscript {
                     node: wrapped_ms,
diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs
@@ -68,8 +68,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
     /// Display code of type_check.
     pub fn from_ast(t: Terminal<Pk, Ctx>) -> Result<Miniscript<Pk, Ctx>, Error> {
         let res = Miniscript {
-            ty: Type::type_check(&t, |_| None)?,
-            ext: ExtData::type_check(&t, |_| None)?,
+            ty: Type::type_check(&t)?,
+            ext: ExtData::type_check(&t)?,
             node: t,
             phantom: PhantomData,
         };
@@ -269,7 +269,7 @@ impl<Ctx: ScriptContext> Miniscript<Ctx::Key, Ctx> {
 
         let top = decode::parse(&mut iter)?;
         Ctx::check_global_validity(&top)?;
-        let type_check = types::Type::type_check(&top.node, |_| None)?;
+        let type_check = types::Type::type_check(&top.node)?;
         if type_check.corr.base != types::Base::B {
             return Err(Error::NonTopLevel(format!("{:?}", top)));
         };
diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs
@@ -888,14 +888,22 @@ impl Property for ExtData {
         })
     }
 
+    fn type_check_with_child<Pk, Ctx, C>(
+        _fragment: &Terminal<Pk, Ctx>,
+        mut _child: C,
+    ) -> Result<Self, Error<Pk, Ctx>>
+    where
+        C: FnMut(usize) -> Self,
+        Pk: MiniscriptKey,
+        Ctx: ScriptContext,
+    {
+        unreachable!()
+    }
+
     /// Compute the type of a fragment assuming all the children of
     /// Miniscript have been computed already.
-    fn type_check<Pk, Ctx, C>(
-        fragment: &Terminal<Pk, Ctx>,
-        _child: C,
-    ) -> Result<Self, Error<Pk, Ctx>>
+    fn type_check<Pk, Ctx>(fragment: &Terminal<Pk, Ctx>) -> Result<Self, Error<Pk, Ctx>>
     where
-        C: FnMut(usize) -> Option<Self>,
         Ctx: ScriptContext,
         Pk: MiniscriptKey,
     {
diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs
@@ -20,12 +20,6 @@ pub use self::malleability::{Dissat, Malleability};
 use super::ScriptContext;
 use crate::{MiniscriptKey, Terminal};
 
-/// None-returning function to help type inference when we need a
-/// closure that simply returns `None`
-fn return_none<T>(_: usize) -> Option<T> {
-    None
-}
-
 /// Detailed type of a typechecker error
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub enum ErrorKind {
@@ -374,20 +368,15 @@ pub trait Property: Sized {
     /// Compute the type of a fragment, given a function to look up
     /// the types of its children, if available and relevant for the
     /// given fragment
-    fn type_check<Pk, Ctx, C>(
-        fragment: &Terminal<Pk, Ctx>,
-        mut child: C,
+    fn type_check_common<'a, Pk, Ctx, C>(
+        fragment: &'a Terminal<Pk, Ctx>,
+        mut get_child: C,
     ) -> Result<Self, Error<Pk, Ctx>>
     where
-        C: FnMut(usize) -> Option<Self>,
+        C: FnMut(&'a Terminal<Pk, Ctx>, usize) -> Result<Self, Error<Pk, Ctx>>,
         Pk: MiniscriptKey,
         Ctx: ScriptContext,
     {
-        let mut get_child = |sub, n| {
-            child(n)
-                .map(Ok)
-                .unwrap_or_else(|| Self::type_check(sub, return_none))
-        };
         let wrap_err = |result: Result<Self, ErrorKind>| {
             result.map_err(|kind| Error {
                 fragment: fragment.clone(),
@@ -523,6 +512,30 @@ pub trait Property: Sized {
         }
         ret
     }
+
+    /// Compute the type of a fragment, given a function to look up
+    /// the types of its children.
+    fn type_check_with_child<Pk, Ctx, C>(
+        fragment: &Terminal<Pk, Ctx>,
+        mut child: C,
+    ) -> Result<Self, Error<Pk, Ctx>>
+    where
+        C: FnMut(usize) -> Self,
+        Pk: MiniscriptKey,
+        Ctx: ScriptContext,
+    {
+        let get_child = |_sub, n| Ok(child(n));
+        Self::type_check_common(fragment, get_child)
+    }
+
+    /// Compute the type of a fragment.
+    fn type_check<Pk, Ctx>(fragment: &Terminal<Pk, Ctx>) -> Result<Self, Error<Pk, Ctx>>
+    where
+        Pk: MiniscriptKey,
+        Ctx: ScriptContext,
+    {
+        Self::type_check_common(fragment, |sub, _n| Self::type_check(sub))
+    }
 }
 
 impl Property for Type {
@@ -767,14 +780,22 @@ impl Property for Type {
         })
     }
 
+    fn type_check_with_child<Pk, Ctx, C>(
+        _fragment: &Terminal<Pk, Ctx>,
+        mut _child: C,
+    ) -> Result<Self, Error<Pk, Ctx>>
+    where
+        C: FnMut(usize) -> Self,
+        Pk: MiniscriptKey,
+        Ctx: ScriptContext,
+    {
+        unreachable!()
+    }
+
     /// Compute the type of a fragment assuming all the children of
     /// Miniscript have been computed already.
-    fn type_check<Pk, Ctx, C>(
-        fragment: &Terminal<Pk, Ctx>,
-        _child: C,
-    ) -> Result<Self, Error<Pk, Ctx>>
+    fn type_check<Pk, Ctx>(fragment: &Terminal<Pk, Ctx>) -> Result<Self, Error<Pk, Ctx>>
     where
-        C: FnMut(usize) -> Option<Self>,
         Pk: MiniscriptKey,
         Ctx: ScriptContext,
     {
diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs
@@ -480,7 +480,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> {
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> {
     fn terminal(ast: Terminal<Pk, Ctx>) -> AstElemExt<Pk, Ctx> {
         AstElemExt {
-            comp_ext_data: CompilerExtData::type_check(&ast, |_| None).unwrap(),
+            comp_ext_data: CompilerExtData::type_check(&ast).unwrap(),
             ms: Arc::new(Miniscript::from_ast(ast).expect("Terminal creation must always succeed")),
         }
     }
@@ -491,15 +491,15 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> {
         r: &AstElemExt<Pk, Ctx>,
     ) -> Result<AstElemExt<Pk, Ctx>, types::Error<Pk, Ctx>> {
         let lookup_ext = |n| match n {
-            0 => Some(l.comp_ext_data),
-            1 => Some(r.comp_ext_data),
+            0 => l.comp_ext_data,
+            1 => r.comp_ext_data,
             _ => unreachable!(),
         };
         //Types and ExtData are already cached and stored in children. So, we can
         //type_check without cache. For Compiler extra data, we supply a cache.
-        let ty = types::Type::type_check(&ast, |_| None)?;
-        let ext = types::ExtData::type_check(&ast, |_| None)?;
-        let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?;
+        let ty = types::Type::type_check(&ast)?;
+        let ext = types::ExtData::type_check(&ast)?;
+        let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?;
         Ok(AstElemExt {
             ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)),
             comp_ext_data,
@@ -513,16 +513,16 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> {
         c: &AstElemExt<Pk, Ctx>,
     ) -> Result<AstElemExt<Pk, Ctx>, types::Error<Pk, Ctx>> {
         let lookup_ext = |n| match n {
-            0 => Some(a.comp_ext_data),
-            1 => Some(b.comp_ext_data),
-            2 => Some(c.comp_ext_data),
+            0 => a.comp_ext_data,
+            1 => b.comp_ext_data,
+            2 => c.comp_ext_data,
             _ => unreachable!(),
         };
         //Types and ExtData are already cached and stored in children. So, we can
         //type_check without cache. For Compiler extra data, we supply a cache.
-        let ty = types::Type::type_check(&ast, |_| None)?;
-        let ext = types::ExtData::type_check(&ast, |_| None)?;
-        let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?;
+        let ty = types::Type::type_check(&ast)?;
+        let ext = types::ExtData::type_check(&ast)?;
+        let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?;
         Ok(AstElemExt {
             ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)),
             comp_ext_data,
