From e65a2c928022297ebb31c1b73624da54f04ff21b Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jsharp@fastly.com>
Date: Fri, 6 Jan 2023 14:00:50 -0800
Subject: [PATCH] cranelift-isle: Record all binding names (#5538)

...not just the ones at the outer scope of a rule.

Thanks to @avanhatt for pointing out that #5221 didn't capture as much
information as I intended it to.
---
 cranelift/isle/isle/src/sema.rs | 43 +++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/cranelift/isle/isle/src/sema.rs b/cranelift/isle/isle/src/sema.rs
index cd467c7f8a..95481f1067 100644
--- a/cranelift/isle/isle/src/sema.rs
+++ b/cranelift/isle/isle/src/sema.rs
@@ -484,6 +484,9 @@ pub struct BoundVar {
     pub name: Sym,
     /// The type of the value this variable is bound to.
     pub ty: TypeId,
+    /// A counter used to check whether this variable is still in scope during
+    /// semantic analysis. Not meaningful afterward.
+    scope: usize,
 }
 
 /// An `if-let` clause with a subpattern match on an expr after the
@@ -1112,19 +1115,46 @@ impl TypeEnv {
 
 #[derive(Clone, Debug, Default)]
 struct Bindings {
+    /// All bindings accumulated so far within the current rule, including let-
+    /// bindings which have gone out of scope.
     seen: Vec<BoundVar>,
+    /// Counter for unique scope IDs within this set of bindings.
+    next_scope: usize,
+    /// Stack of the scope IDs for bindings which are currently in scope.
+    in_scope: Vec<usize>,
 }
 
 impl Bindings {
+    fn enter_scope(&mut self) {
+        self.in_scope.push(self.next_scope);
+        self.next_scope += 1;
+    }
+
+    fn exit_scope(&mut self) {
+        self.in_scope.pop();
+    }
+
     fn add_var(&mut self, name: Sym, ty: TypeId) -> VarId {
         let id = VarId(self.seen.len());
-        log!("binding var {:?} as {:?} with type {:?}", name.0, id, ty);
-        self.seen.push(BoundVar { id, name, ty });
+        let var = BoundVar {
+            id,
+            name,
+            ty,
+            scope: *self
+                .in_scope
+                .last()
+                .expect("enter_scope should be called before add_var"),
+        };
+        log!("binding var {:?}", var);
+        self.seen.push(var);
         id
     }
 
     fn lookup(&self, name: Sym) -> Option<&BoundVar> {
-        self.seen.iter().rev().find(|binding| binding.name == name)
+        self.seen
+            .iter()
+            .rev()
+            .find(|binding| binding.name == name && self.in_scope.contains(&binding.scope))
     }
 }
 
@@ -1656,6 +1686,7 @@ impl TermEnv {
                 &ast::Def::Rule(ref rule) => {
                     let pos = rule.pos;
                     let mut bindings = Bindings::default();
+                    bindings.enter_scope();
 
                     let (sym, args) = if let ast::Pattern::Term { sym, args, .. } = &rule.pattern {
                         (sym, args)
@@ -1711,6 +1742,8 @@ impl TermEnv {
                         /* on_lhs */ false,
                     ));
 
+                    bindings.exit_scope();
+
                     let rid = RuleId(self.rules.len());
                     self.rules.push(Rule {
                         id: rid,
@@ -2299,7 +2332,7 @@ impl TermEnv {
                 ref body,
                 pos,
             } => {
-                let orig_binding_len = bindings.seen.len();
+                bindings.enter_scope();
 
                 // For each new binding...
                 let mut let_defs = vec![];
@@ -2340,7 +2373,7 @@ impl TermEnv {
                 let body_ty = body.ty();
 
                 // Pop the bindings.
-                bindings.seen.truncate(orig_binding_len);
+                bindings.exit_scope();
 
                 Some(Expr::Let {
                     ty: body_ty,