expression: rewrite Tree module to no longer use a recursive data type

This significantly speeds up and simplifies tree parsing, at the cost of
having a more complicated API (but we mostly addressed the API question
in the previous commits).

This completely eliminates recursion for the Tree data type, including
in the Drop impl.

Big diff but there are only two "real" changes -- expression/mod.rs is
substantially rewritten of course since we replace the core datatype,
and Tr::from_tree is substantially rewritten since doing so was the
point of this change. The rest of the changes are mechanically changing
the signature of expression::FromTree::from_tree everywhere.

src/descriptor/bare.rs

@@ -175,8 +175,8 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Bare<Pk> {
 }
 
 impl<Pk: FromStrKey> FromTree for Bare<Pk> {
-    fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
-        let sub = Miniscript::<Pk, BareCtx>::from_tree(top)?;
+    fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
+        let sub = Miniscript::<Pk, BareCtx>::from_tree(root)?;
         BareCtx::top_level_checks(&sub)?;
         Bare::new(sub)
     }
@@ -186,7 +186,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Bare<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let top = expression::Tree::from_str(s)?;
-        Self::from_tree(&top)
+        Self::from_tree(top.root())
     }
 }
 
@@ -369,8 +369,8 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Pkh<Pk> {
 }
 
 impl<Pk: FromStrKey> FromTree for Pkh<Pk> {
-    fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
-        let pk = top
+    fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
+        let pk = root
             .verify_terminal_parent("pkh", "public key")
             .map_err(Error::Parse)?;
         Pkh::new(pk).map_err(Error::ContextError)
@@ -381,7 +381,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Pkh<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let top = expression::Tree::from_str(s)?;
-        Self::from_tree(&top)
+        Self::from_tree(top.root())
     }
 }
 

src/descriptor/mod.rs

@@ -965,7 +965,7 @@ impl Descriptor<DefiniteDescriptorKey> {
 
 impl<Pk: FromStrKey> crate::expression::FromTree for Descriptor<Pk> {
     /// Parse an expression tree into a descriptor.
-    fn from_tree(top: &expression::Tree) -> Result<Descriptor<Pk>, Error> {
+    fn from_tree(top: expression::TreeIterItem) -> Result<Descriptor<Pk>, Error> {
         Ok(match (top.name(), top.n_children()) {
             ("pkh", 1) => Descriptor::Pkh(Pkh::from_tree(top)?),
             ("wpkh", 1) => Descriptor::Wpkh(Wpkh::from_tree(top)?),
@@ -981,7 +981,7 @@ impl<Pk: FromStrKey> FromStr for Descriptor<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Descriptor<Pk>, Error> {
         let top = expression::Tree::from_str(s)?;
-        let ret = Self::from_tree(&top)?;
+        let ret = Self::from_tree(top.root())?;
         if let Descriptor::Tr(ref inner) = ret {
             // FIXME preserve weird/broken behavior from 12.x.
             // See https://github.com/rust-bitcoin/rust-miniscript/issues/734

src/descriptor/segwitv0.rs

@@ -247,7 +247,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wsh<Pk> {
 }
 
 impl<Pk: FromStrKey> crate::expression::FromTree for Wsh<Pk> {
-    fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
+    fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
         let top = top
             .verify_toplevel("wsh", 1..=1)
             .map_err(From::from)
@@ -284,7 +284,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Wsh<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let top = expression::Tree::from_str(s)?;
-        Wsh::<Pk>::from_tree(&top)
+        Wsh::<Pk>::from_tree(top.root())
     }
 }
 
@@ -483,7 +483,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wpkh<Pk> {
 }
 
 impl<Pk: FromStrKey> crate::expression::FromTree for Wpkh<Pk> {
-    fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
+    fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
         let pk = top
             .verify_terminal_parent("wpkh", "public key")
             .map_err(Error::Parse)?;
@@ -495,7 +495,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Wpkh<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let top = expression::Tree::from_str(s)?;
-        Self::from_tree(&top)
+        Self::from_tree(top.root())
     }
 }
 

src/descriptor/sh.rs

@@ -81,7 +81,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
 }
 
 impl<Pk: FromStrKey> crate::expression::FromTree for Sh<Pk> {
-    fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
+    fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
         let top = top
             .verify_toplevel("sh", 1..=1)
             .map_err(From::from)
@@ -105,7 +105,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Sh<Pk> {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let top = expression::Tree::from_str(s)?;
-        Self::from_tree(&top)
+        Self::from_tree(top.root())
     }
 }
 

src/descriptor/sortedmulti.rs

@@ -59,7 +59,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
     }
 
     /// Parse an expression tree into a SortedMultiVec
-    pub fn from_tree(tree: &expression::Tree) -> Result<Self, Error>
+    pub fn from_tree(tree: expression::TreeIterItem) -> Result<Self, Error>
     where
         Pk: FromStrKey,
     {

src/descriptor/tr.rs

@@ -14,7 +14,6 @@ use sync::Arc;
 use super::checksum;
 use crate::descriptor::DefiniteDescriptorKey;
 use crate::expression::{self, FromTree};
-use crate::iter::TreeLike as _;
 use crate::miniscript::satisfy::{Placeholder, Satisfaction, SchnorrSigType, Witness};
 use crate::miniscript::Miniscript;
 use crate::plan::AssetProvider;
@@ -495,99 +494,84 @@ impl<Pk: FromStrKey> core::str::FromStr for Tr<Pk> {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let expr_tree = expression::Tree::from_str(s)?;
-        Self::from_tree(&expr_tree)
+        Self::from_tree(expr_tree.root())
     }
 }
 
 impl<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
-    fn from_tree(expr_tree: &expression::Tree) -> Result<Self, Error> {
+    fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
         use crate::expression::{Parens, ParseTreeError};
 
-        expr_tree
-            .verify_toplevel("tr", 1..=2)
+        struct TreeStack<'s, Pk: MiniscriptKey> {
+            inner: Vec<(expression::TreeIterItem<'s>, TapTree<Pk>)>,
+        }
+
+        impl<'s, Pk: MiniscriptKey> TreeStack<'s, Pk> {
+            fn new() -> Self { Self { inner: Vec::with_capacity(128) } }
+
+            fn push(&mut self, parent: expression::TreeIterItem<'s>, tree: TapTree<Pk>) {
+                let mut next_push = (parent, tree);
+                while let Some(top) = self.inner.pop() {
+                    if next_push.0.index() == top.0.index() {
+                        next_push.0 = top.0.parent().unwrap();
+                        next_push.1 = TapTree::combine(top.1, next_push.1);
+                    } else {
+                        self.inner.push(top);
+                        break;
+                    }
+                }
+                self.inner.push(next_push);
+            }
+
+            fn pop_final(&mut self) -> Option<TapTree<Pk>> {
+                assert_eq!(self.inner.len(), 1);
+                self.inner.pop().map(|x| x.1)
+            }
+        }
+
+        root.verify_toplevel("tr", 1..=2)
             .map_err(From::from)
             .map_err(Error::Parse)?;
 
-        let mut round_paren_depth = 0;
+        let mut root_children = root.children();
+        let internal_key: Pk = root_children
+            .next()
+            .unwrap() // `verify_toplevel` above checked that first child existed
+            .verify_terminal("internal key")
+            .map_err(Error::Parse)?;
 
-        let mut internal_key = None;
-        let mut tree_stack = vec![];
+        let tap_tree = match root_children.next() {
+            None => return Tr::new(internal_key, None),
+            Some(tree) => tree,
+        };
 
-        for item in expr_tree.verbose_pre_order_iter() {
-            // Top-level "tr" node.
-            if item.index == 0 {
-                if item.is_complete {
-                    debug_assert!(
-                        internal_key.is_some(),
-                        "checked above that top-level 'tr' has children"
-                    );
+        let mut tree_stack = TreeStack::new();
+        let mut tap_tree_iter = tap_tree.pre_order_iter();
+        // while let construction needed because we modify the iterator inside the loop
+        // (by calling skip_descendants to skip over the contents of the tapscripts).
+        while let Some(node) = tap_tree_iter.next() {
+            if node.parens() == Parens::Curly {
+                if !node.name().is_empty() {
+                    return Err(Error::Parse(ParseError::Tree(ParseTreeError::IncorrectName {
+                        actual: node.name().to_owned(),
+                        expected: "",
+                    })));
                 }
-            } else if item.index == 1 {
-                // First child of tr, which must be the internal key
-                internal_key = item
-                    .node
-                    .verify_terminal("internal key")
-                    .map_err(Error::Parse)
-                    .map(Some)?;
+                node.verify_n_children("taptree branch", 2..=2)
+                    .map_err(From::from)
+                    .map_err(Error::Parse)?;
             } else {
-                // From here on we are into the taptree.
-                if item.n_children_yielded == 0 {
-                    match item.node.parens() {
-                        Parens::Curly => {
-                            if !item.node.name().is_empty() {
-                                return Err(Error::Parse(ParseError::Tree(
-                                    ParseTreeError::IncorrectName {
-                                        actual: item.node.name().to_owned(),
-                                        expected: "",
-                                    },
-                                )));
-                            }
-                            if round_paren_depth > 0 {
-                                return Err(Error::Parse(ParseError::Tree(
-                                    ParseTreeError::IllegalCurlyBrace {
-                                        pos: item.node.children_pos(),
-                                    },
-                                )));
-                            }
-                        }
-                        Parens::Round => round_paren_depth += 1,
-                        _ => {}
-                    }
-                }
-                if item.is_complete {
-                    if item.node.parens() == Parens::Curly {
-                        if item.n_children_yielded == 2 {
-                            let rchild = tree_stack.pop().unwrap();
-                            let lchild = tree_stack.pop().unwrap();
-                            tree_stack.push(TapTree::combine(lchild, rchild));
-                        } else {
-                            return Err(Error::Parse(ParseError::Tree(
-                                ParseTreeError::IncorrectNumberOfChildren {
-                                    description: "Taptree node",
-                                    n_children: item.n_children_yielded,
-                                    minimum: Some(2),
-                                    maximum: Some(2),
-                                },
-                            )));
-                        }
-                    } else {
-                        if item.node.parens() == Parens::Round {
-                            round_paren_depth -= 1;
-                        }
-                        if round_paren_depth == 0 {
-                            let script = Miniscript::from_tree(item.node)?;
-                            // FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734
-                            if script.ty.corr.base != crate::miniscript::types::Base::B {
-                                return Err(Error::NonTopLevel(format!("{:?}", script)));
-                            };
-                            tree_stack.push(TapTree::Leaf(Arc::new(script)));
-                        }
-                    }
-                }
+                let script = Miniscript::from_tree(node)?;
+                // FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734
+                if script.ty.corr.base != crate::miniscript::types::Base::B {
+                    return Err(Error::NonTopLevel(format!("{:?}", script)));
+                };
+
+                tree_stack.push(node.parent().unwrap(), TapTree::Leaf(Arc::new(script)));
+                tap_tree_iter.skip_descendants();
             }
         }
-        assert!(tree_stack.len() <= 1);
-        Tr::new(internal_key.unwrap(), tree_stack.pop())
+        Tr::new(internal_key, tree_stack.pop_final())
     }
 }