ForEachKey: clean up Concrete impl, remove Semantic impl

The Concrete impl used a weird complier gitch `let pred = |x| pred(x)`
to avoid an infinite recursion bug (which in turn triggered a compiler
bug that they didn't fix even though I filed it a DAY AFTER IT WAS
INTRODUCED ON NIGHTLY and instead they waited 3 months and released it
on stable forcing me to backport a workaround to a gazillion versions of
rust-miniscript).

The bug was rust-lang/rust#110475 BTW, just to
make sure this commit triggers another notification on that issue.

Clippy doesn't like this. Clippy is obviously wrong but rather than
having this stupid fight again just switch to a different idiom.

Meanwhile, the Semantic impl of ForEachKey panics iff any keys are
present, regardless of the closure passed to it. This is funny but
completely useless and we should just remove it.

Author: apoelstra

src/policy/concrete.rs

@@ -662,7 +662,12 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Policy<Pk> {
     where
         Pk: 'a,
     {
-        let mut pred = |x| pred(x);
+        self.for_each_key_internal(&mut pred)
+    }
+}
+
+impl<Pk: MiniscriptKey> Policy<Pk> {
+    fn for_each_key_internal<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: &mut F) -> bool {
         match *self {
             Policy::Unsatisfiable | Policy::Trivial => true,
             Policy::Key(ref pk) => pred(pk),
@@ -673,14 +678,12 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Policy<Pk> {
             | Policy::After(..)
             | Policy::Older(..) => true,
             Policy::Threshold(_, ref subs) | Policy::And(ref subs) => {
-                subs.iter().all(|sub| sub.for_each_key(&mut pred))
+                subs.iter().all(|sub| sub.for_each_key_internal(pred))
             }
-            Policy::Or(ref subs) => subs.iter().all(|(_, sub)| sub.for_each_key(&mut pred)),
+            Policy::Or(ref subs) => subs.iter().all(|(_, sub)| sub.for_each_key_internal(pred)),
         }
     }
-}
 
-impl<Pk: MiniscriptKey> Policy<Pk> {
     /// Convert a policy using one kind of public key to another
     /// type of public key
     ///

src/policy/semantic.rs

@@ -10,7 +10,7 @@ use elements::{LockTime, Sequence};
 
 use super::concrete::PolicyError;
 use super::ENTAILMENT_MAX_TERMINALS;
-use crate::{errstr, expression, AbsLockTime, Error, ForEachKey, MiniscriptKey, Translator};
+use crate::{errstr, expression, AbsLockTime, Error, MiniscriptKey, Translator};
 
 /// Abstract policy which corresponds to the semantics of a Miniscript
 /// and which allows complex forms of analysis, e.g. filtering and
@@ -59,26 +59,6 @@ where
     }
 }
 
-impl<Pk: MiniscriptKey> ForEachKey<Pk> for Policy<Pk> {
-    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
-    where
-        Pk: 'a,
-    {
-        let mut pred = |x| pred(x);
-        match *self {
-            Policy::Unsatisfiable | Policy::Trivial => true,
-            Policy::Key(ref _pkh) => todo!("Semantic Policy KeyHash must store Pk"),
-            Policy::Sha256(..)
-            | Policy::Hash256(..)
-            | Policy::Ripemd160(..)
-            | Policy::Hash160(..)
-            | Policy::After(..)
-            | Policy::Older(..) => true,
-            Policy::Threshold(_, ref subs) => subs.iter().all(|sub| sub.for_each_key(&mut pred)),
-        }
-    }
-}
-
 impl<Pk: MiniscriptKey> Policy<Pk> {
     /// Convert a policy using one kind of public key to another
     /// type of public key