Move variables to var stack (#8604)

# Description

This moves the representation of variables on the stack to a Vec, which
more closely resembles a stack. For small numbers of variables live at
any one point, this tends to be more efficient than a HashMap. Having a
stack-like vector also allows us to remember a stack position,
temporarily push variables on, then quickly drop the stack back to the
original size when we're done. We'll need this capability to allow
matching inside of conditions.

On this mac, a simple run of:

`timeit { mut x = 1; while $x < 1000000 { $x += 1 } }`

Went from 1 sec 86 ms, down to 1 sec 2 ms. Clearly, we have a lot more
ground we can make up in looping speed 😅 but it's nice that for fixing
this to make matching easier, we also get a win in terms of lookup speed
for small numbers of variables.

# User-Facing Changes

Likely users won't (hopefully) see any negative impact and may even see
a small positive impact.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

crates/nu-cmd-lang/src/core_commands/collect.rs

@@ -72,10 +72,10 @@ impl Command for Collect {
             // for when we support `data | let x = $in;`
             // remove the variables added earlier
             for var_id in capture_block.captures.keys() {
-                stack_captures.vars.remove(var_id);
+                stack_captures.remove_var(*var_id);
             }
             if let Some(u) = saved_positional {
-                stack_captures.vars.remove(&u);
+                stack_captures.remove_var(u);
             }
             // add any new variables to the stack
             stack.vars.extend(stack_captures.vars);

crates/nu-cmd-lang/src/core_commands/match_.rs

@@ -113,7 +113,7 @@ impl Command for Match {
             },
             Example {
                 description: "Match against pipeline input",
-                example: "{a: {b: 3}} | match $in.a { { $b } => ($b + 10) }",
+                example: "{a: {b: 3}} | match $in {{a: { $b }} => ($b + 10) }",
                 result: Some(Value::test_int(13)),
             },
         ]

crates/nu-command/src/hook.rs

@@ -202,7 +202,7 @@ pub fn eval_hook(
                         }
 
                         for var_id in var_ids.iter() {
-                            stack.vars.remove(var_id);
+                            stack.remove_var(*var_id);
                         }
                     }
                     Value::Block {

crates/nu-engine/src/eval.rs

@@ -459,7 +459,7 @@ pub fn eval_expression(
                         Expr::Var(var_id) | Expr::VarDecl(var_id) => {
                             let var_info = engine_state.get_var(*var_id);
                             if var_info.mutable {
-                                stack.vars.insert(*var_id, rhs);
+                                stack.add_var(*var_id, rhs);
                                 Ok(Value::nothing(lhs.span))
                             } else {
                                 Err(ShellError::AssignmentRequiresMutableVar { lhs_span: lhs.span })
@@ -499,7 +499,7 @@ pub fn eval_expression(
                                             }
                                         }
                                     } else {
-                                        stack.vars.insert(*var_id, lhs);
+                                        stack.add_var(*var_id, lhs);
                                     }
                                     Ok(Value::nothing(cell_path.head.span))
                                 } else {

crates/nu-protocol/src/engine/stack.rs

@@ -65,7 +65,7 @@ impl ProfilingConfig {
 #[derive(Debug, Clone)]
 pub struct Stack {
     /// Variables
-    pub vars: HashMap<VarId, Value>,
+    pub vars: Vec<(VarId, Value)>,
     /// Environment variables arranged as a stack to be able to recover values from parent scopes
     pub env_vars: Vec<EnvVars>,
     /// Tells which environment variables from engine state are hidden, per overlay.
@@ -79,7 +79,7 @@ pub struct Stack {
 impl Stack {
     pub fn new() -> Stack {
         Stack {
-            vars: HashMap::new(),
+            vars: vec![],
             env_vars: vec![],
             env_hidden: HashMap::new(),
             active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()],
@@ -104,23 +104,43 @@ impl Stack {
     }
 
     pub fn get_var(&self, var_id: VarId, span: Span) -> Result<Value, ShellError> {
-        if let Some(v) = self.vars.get(&var_id) {
-            return Ok(v.clone().with_span(span));
+        for (id, val) in &self.vars {
+            if var_id == *id {
+                return Ok(val.clone().with_span(span));
+            }
         }
 
         Err(ShellError::VariableNotFoundAtRuntime { span })
     }
 
     pub fn get_var_with_origin(&self, var_id: VarId, span: Span) -> Result<Value, ShellError> {
-        if let Some(v) = self.vars.get(&var_id) {
-            return Ok(v.clone());
+        for (id, val) in &self.vars {
+            if var_id == *id {
+                return Ok(val.clone());
+            }
         }
 
         Err(ShellError::VariableNotFoundAtRuntime { span })
     }
 
     pub fn add_var(&mut self, var_id: VarId, value: Value) {
-        self.vars.insert(var_id, value);
+        //self.vars.insert(var_id, value);
+        for (id, val) in &mut self.vars {
+            if *id == var_id {
+                *val = value;
+                return;
+            }
+        }
+        self.vars.push((var_id, value));
+    }
+
+    pub fn remove_var(&mut self, var_id: VarId) {
+        for (idx, (id, _)) in self.vars.iter().enumerate() {
+            if *id == var_id {
+                self.vars.remove(idx);
+                return;
+            }
+        }
     }
 
     pub fn add_env_var(&mut self, var: String, value: Value) {
@@ -162,8 +182,14 @@ impl Stack {
         let mut env_vars = self.env_vars.clone();
         env_vars.push(HashMap::new());
 
+        // FIXME make this more efficient
+        let mut vars = vec![];
+        for (id, val) in captures {
+            vars.push((*id, val.clone()));
+        }
+
         Stack {
-            vars: captures.clone(),
+            vars,
             env_vars,
             env_hidden: self.env_hidden.clone(),
             active_overlays: self.active_overlays.clone(),
@@ -173,7 +199,7 @@ impl Stack {
     }
 
     pub fn gather_captures(&self, captures: &[VarId]) -> Stack {
-        let mut vars = HashMap::new();
+        let mut vars = vec![];
 
         let fake_span = Span::new(0, 0);
 
@@ -181,7 +207,7 @@ impl Stack {
             // Note: this assumes we have calculated captures correctly and that commands
             // that take in a var decl will manually set this into scope when running the blocks
             if let Ok(value) = self.get_var(*capture, fake_span) {
-                vars.insert(*capture, value);
+                vars.push((*capture, value));
             }
         }