Some fixes to the internal-extractor macro substitution

cranelift/isle/src/ast.rs

@@ -99,6 +99,8 @@ pub enum Pattern {
     Wildcard,
     /// N sub-patterns that must all match.
     And { subpats: Vec<Pattern> },
+    /// Internal use only: macro argument in a template.
+    MacroArg { index: usize },
 }
 
 impl Pattern {
@@ -109,6 +111,81 @@ impl Pattern {
             _ => None,
         }
     }
+
+    pub fn make_macro_template(&self, macro_args: &[Ident]) -> Pattern {
+        log::trace!("repplace_macro_args: {:?} with {:?}", self, macro_args);
+        match self {
+            &Pattern::BindPattern {
+                ref var,
+                ref subpat,
+            } if matches!(&**subpat, &Pattern::Wildcard) => {
+                if let Some(i) = macro_args.iter().position(|arg| arg == var) {
+                    Pattern::MacroArg { index: i }
+                } else {
+                    self.clone()
+                }
+            }
+            &Pattern::BindPattern {
+                ref var,
+                ref subpat,
+            } => Pattern::BindPattern {
+                var: var.clone(),
+                subpat: Box::new(subpat.make_macro_template(macro_args)),
+            },
+            &Pattern::And { ref subpats } => {
+                let subpats = subpats
+                    .iter()
+                    .map(|subpat| subpat.make_macro_template(macro_args))
+                    .collect::<Vec<_>>();
+                Pattern::And { subpats }
+            }
+            &Pattern::Term { ref sym, ref args } => {
+                let args = args
+                    .iter()
+                    .map(|arg| arg.make_macro_template(macro_args))
+                    .collect::<Vec<_>>();
+                Pattern::Term {
+                    sym: sym.clone(),
+                    args,
+                }
+            }
+
+            &Pattern::Var { .. } | &Pattern::Wildcard | &Pattern::ConstInt { .. } => self.clone(),
+            &Pattern::MacroArg { .. } => unreachable!(),
+        }
+    }
+
+    pub fn subst_macro_args(&self, macro_args: &[Pattern]) -> Pattern {
+        match self {
+            &Pattern::BindPattern {
+                ref var,
+                ref subpat,
+            } => Pattern::BindPattern {
+                var: var.clone(),
+                subpat: Box::new(subpat.subst_macro_args(macro_args)),
+            },
+            &Pattern::And { ref subpats } => {
+                let subpats = subpats
+                    .iter()
+                    .map(|subpat| subpat.subst_macro_args(macro_args))
+                    .collect::<Vec<_>>();
+                Pattern::And { subpats }
+            }
+            &Pattern::Term { ref sym, ref args } => {
+                let args = args
+                    .iter()
+                    .map(|arg| arg.subst_macro_args(macro_args))
+                    .collect::<Vec<_>>();
+                Pattern::Term {
+                    sym: sym.clone(),
+                    args,
+                }
+            }
+
+            &Pattern::Var { .. } | &Pattern::Wildcard | &Pattern::ConstInt { .. } => self.clone(),
+            &Pattern::MacroArg { index } => macro_args[index].clone(),
+        }
+    }
 }
 
 /// A pattern in a term argument. Adds "evaluated expression" to kinds
@@ -125,6 +202,27 @@ pub enum TermArgPattern {
     Expr(Expr),
 }
 
+impl TermArgPattern {
+    fn make_macro_template(&self, args: &[Ident]) -> TermArgPattern {
+        log::trace!("repplace_macro_args: {:?} with {:?}", self, args);
+        match self {
+            &TermArgPattern::Pattern(ref pat) => {
+                TermArgPattern::Pattern(pat.make_macro_template(args))
+            }
+            &TermArgPattern::Expr(_) => self.clone(),
+        }
+    }
+
+    fn subst_macro_args(&self, args: &[Pattern]) -> TermArgPattern {
+        match self {
+            &TermArgPattern::Pattern(ref pat) => {
+                TermArgPattern::Pattern(pat.subst_macro_args(args))
+            }
+            &TermArgPattern::Expr(_) => self.clone(),
+        }
+    }
+}
+
 /// An expression: the right-hand side of a rule.
 ///
 /// Note that this *almost* looks like a core Lisp or lambda calculus,

cranelift/isle/src/sema.rs

@@ -110,10 +110,7 @@ pub enum TermKind {
     /// A term that defines an "extractor macro" in the LHS of a
     /// pattern. Its arguments take patterns and are simply
     /// substituted with the given patterns when used.
-    InternalExtractor {
+    InternalExtractor { template: ast::Pattern },
-        args: Vec<ast::Ident>,
-        template: ast::Pattern,
-    },
     /// A term defined solely by an external extractor function.
     ExternalExtractor {
         /// Extractor func.
@@ -605,12 +602,11 @@ impl TermEnv {
                         )
                     })?;
                     let termdata = &mut self.terms[term.index()];
+                    let template = ext.template.make_macro_template(&ext.args[..]);
+                    log::trace!("extractor def: {:?} becomes template {:?}", def, template);
                     match &termdata.kind {
                         &TermKind::Declared => {
-                            termdata.kind = TermKind::InternalExtractor {
+                            termdata.kind = TermKind::InternalExtractor { template };
-                                args: ext.args.clone(),
-                                template: ext.template.clone(),
-                            };
                         }
                         _ => {
                             return Err(tyenv.error(
@@ -644,7 +640,6 @@ impl TermEnv {
                         &rule.pattern,
                         None,
                         &mut bindings,
-                        None,
                     )?;
                     let rhs =
                         self.translate_expr(tyenv, rule.pos, &rule.expr, ty, &mut bindings)?;
@@ -750,7 +745,6 @@ impl TermEnv {
         pat: &ast::Pattern,
         expected_ty: Option<TypeId>,
         bindings: &mut Bindings,
-        macro_args: Option<&HashMap<ast::Ident, ast::Pattern>>,
     ) -> SemaResult<(Pattern, TypeId)> {
         log::trace!("translate_pattern: {:?}", pat);
         log::trace!("translate_pattern: bindings = {:?}", bindings);
@@ -772,14 +766,8 @@ impl TermEnv {
                 let mut expected_ty = expected_ty;
                 let mut children = vec![];
                 for subpat in subpats {
-                    let (subpat, ty) = self.translate_pattern(
+                    let (subpat, ty) =
-                        tyenv,
+                        self.translate_pattern(tyenv, pos, &*subpat, expected_ty, bindings)?;
-                        pos,
-                        &*subpat,
-                        expected_ty,
-                        bindings,
-                        macro_args,
-                    )?;
                     expected_ty = expected_ty.or(Some(ty));
                     children.push(subpat);
                 }
@@ -793,29 +781,9 @@ impl TermEnv {
                 ref var,
                 ref subpat,
             } => {
-                // Handle macro-arg substitution.
-                if macro_args.is_some() && &**subpat == &ast::Pattern::Wildcard {
-                    if let Some(macro_ast) = macro_args.as_ref().unwrap().get(var) {
-                        return self.translate_pattern(
-                            tyenv,
-                            pos,
-                            macro_ast,
-                            expected_ty,
-                            bindings,
-                            macro_args,
-                        );
-                    }
-                }
-
                 // Do the subpattern first so we can resolve the type for sure.
-                let (subpat, ty) = self.translate_pattern(
+                let (subpat, ty) =
-                    tyenv,
+                    self.translate_pattern(tyenv, pos, &*subpat, expected_ty, bindings)?;
-                    pos,
-                    &*subpat,
-                    expected_ty,
-                    bindings,
-                    macro_args,
-                )?;
 
                 let name = tyenv.intern_mut(var);
                 if bindings.vars.iter().any(|bv| bv.name == name) {
@@ -826,6 +794,7 @@ impl TermEnv {
                 }
                 let id = VarId(bindings.next_var);
                 bindings.next_var += 1;
+                log::trace!("binding var {:?}", var.0);
                 bindings.vars.push(BoundVar { name, id, ty });
 
                 Ok((Pattern::BindPattern(ty, id, Box::new(subpat)), ty))
@@ -916,33 +885,24 @@ impl TermEnv {
                             }
                         }
                     }
-                    &TermKind::InternalExtractor {
+                    &TermKind::InternalExtractor { ref template } => {
-                        args: ref template_args,
-                        ref template,
-                    } => {
                         // Expand the extractor macro! We create a map
                         // from macro args to AST pattern trees and
                         // then evaluate the template with these
                         // substitutions.
-                        let mut arg_map = HashMap::new();
+                        let mut macro_args: Vec<ast::Pattern> = vec![];
-                        for (template_arg, sub_ast) in template_args.iter().zip(args.iter()) {
+                        for template_arg in args {
-                            let sub_ast = match sub_ast {
+                            let sub_ast = match template_arg {
                                 &ast::TermArgPattern::Pattern(ref pat) => pat.clone(),
                                 &ast::TermArgPattern::Expr(_) => {
                                     return Err(tyenv.error(pos, "Cannot expand an extractor macro with an expression in a macro argument".to_string()));
                                 }
                             };
-                            arg_map.insert(template_arg.clone(), sub_ast.clone());
+                            macro_args.push(sub_ast.clone());
                         }
-                        log::trace!("internal extractor map = {:?}", arg_map);
+                        log::trace!("internal extractor macro args = {:?}", args);
-                        return self.translate_pattern(
+                        let pat = template.subst_macro_args(&macro_args[..]);
-                            tyenv,
+                        return self.translate_pattern(tyenv, pos, &pat, expected_ty, bindings);
-                            pos,
-                            template,
-                            expected_ty,
-                            bindings,
-                            Some(&arg_map),
-                        );
                     }
                     &TermKind::ExternalConstructor { .. } | &TermKind::InternalConstructor => {
                         // OK.
@@ -957,19 +917,14 @@ impl TermEnv {
                 let mut subpats = vec![];
                 for (i, arg) in args.iter().enumerate() {
                     let arg_ty = self.terms[tid.index()].arg_tys[i];
-                    let (subpat, _) = self.translate_pattern_term_arg(
+                    let (subpat, _) =
-                        tyenv,
+                        self.translate_pattern_term_arg(tyenv, pos, arg, Some(arg_ty), bindings)?;
-                        pos,
-                        arg,
-                        Some(arg_ty),
-                        bindings,
-                        macro_args,
-                    )?;
                     subpats.push(subpat);
                 }
 
                 Ok((Pattern::Term(ty, *tid, subpats), ty))
             }
+            &ast::Pattern::MacroArg { .. } => unreachable!(),
         }
     }
 
@@ -980,12 +935,11 @@ impl TermEnv {
         pat: &ast::TermArgPattern,
         expected_ty: Option<TypeId>,
         bindings: &mut Bindings,
-        macro_args: Option<&HashMap<ast::Ident, ast::Pattern>>,
     ) -> SemaResult<(TermArgPattern, TypeId)> {
         match pat {
             &ast::TermArgPattern::Pattern(ref pat) => {
                 let (subpat, ty) =
-                    self.translate_pattern(tyenv, pos, pat, expected_ty, bindings, macro_args)?;
+                    self.translate_pattern(tyenv, pos, pat, expected_ty, bindings)?;
                 Ok((TermArgPattern::Pattern(subpat), ty))
             }
             &ast::TermArgPattern::Expr(ref expr) => {
@@ -1010,6 +964,7 @@ impl TermEnv {
         ty: TypeId,
         bindings: &mut Bindings,
     ) -> SemaResult<Expr> {
+        log::trace!("translate_expr: {:?}", expr);
         match expr {
             &ast::Expr::Term { ref sym, ref args } => {
                 // Look up the term.