Merge #584: reduce binary bloat by removing generic param from type_check

0c411db reduce binary bloat by removing generic param from type_check (Marko Bencun)

Pull request description:

  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 && 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.

ACKs for top commit:
  apoelstra:
    ACK 0c411db
  sanket1729:
    ACK 0c411db

Tree-SHA512: 5526076230a3d5140279567813f87764fc09f3fc8e95d031bf98f52726041afdb262af715a3af19ccae2c0c85069460c20bbe4073364b7a4a4e1378602835e29

Author: apoelstra

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",

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,

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)));
         };

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,
     {

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,
     {

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,