Use Vec for Closure captures (#10940)

# Description
Changes the `captures` field in `Closure` from a `HashMap` to a `Vec`
and makes `Stack::captures_to_stack` take an owned `Vec` instead of a
borrowed `HashMap`.

This eliminates the conversion to a `Vec` inside `captures_to_stack` and
makes it possible to avoid clones altogether when using an owned
`Closure` (which is the case for most commands). Additionally, using a
`Vec` reduces the size of `Value` by 8 bytes (down to 72).

# User-Facing Changes
Breaking API change for `nu-protocol`.

crates/nu-cli/src/prompt_update.rs

@@ -42,7 +42,7 @@ fn get_prompt_string(
         .and_then(|v| match v {
             Value::Closure { val, .. } => {
                 let block = engine_state.get_block(val.block_id);
-                let mut stack = stack.captures_to_stack(&val.captures);
+                let mut stack = stack.captures_to_stack(val.captures);
                 // Use eval_subexpression to force a redirection of output, so we can use everything in prompt
                 let ret_val =
                     eval_subexpression(engine_state, &mut stack, block, PipelineData::empty());

crates/nu-cli/src/reedline_config.rs

@@ -252,7 +252,7 @@ pub(crate) fn add_columnar_menu(
             let menu_completer = NuMenuCompleter::new(
                 val.block_id,
                 span,
-                stack.captures_to_stack(&val.captures),
+                stack.captures_to_stack(val.captures.clone()),
                 engine_state,
                 only_buffer_difference,
             );
@@ -334,7 +334,7 @@ pub(crate) fn add_list_menu(
             let menu_completer = NuMenuCompleter::new(
                 val.block_id,
                 span,
-                stack.captures_to_stack(&val.captures),
+                stack.captures_to_stack(val.captures.clone()),
                 engine_state,
                 only_buffer_difference,
             );
@@ -452,7 +452,7 @@ pub(crate) fn add_description_menu(
             let menu_completer = NuMenuCompleter::new(
                 val.block_id,
                 span,
-                stack.captures_to_stack(&val.captures),
+                stack.captures_to_stack(val.captures.clone()),
                 engine_state,
                 only_buffer_difference,
             );

crates/nu-cmd-extra/src/extra/filters/each_while.rs

@@ -81,7 +81,7 @@ impl Command for EachWhile {
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(capture_block.block_id).clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let span = call.head;

crates/nu-cmd-extra/src/extra/filters/update_cells.rs

@@ -96,7 +96,7 @@ impl Command for UpdateCells {
         // the block to run on each cell
         let engine_state = engine_state.clone();
         let block: Closure = call.req(&engine_state, stack, 0)?;
-        let mut stack = stack.captures_to_stack(&block.captures);
+        let mut stack = stack.captures_to_stack(block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
 

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

@@ -44,7 +44,7 @@ impl Command for Collect {
         let capture_block: Closure = call.req(engine_state, stack, 0)?;
 
         let block = engine_state.get_block(capture_block.block_id).clone();
-        let mut stack_captures = stack.captures_to_stack(&capture_block.captures);
+        let mut stack_captures = stack.captures_to_stack(capture_block.captures.clone());
 
         let metadata = input.metadata();
         let input: Value = input.into_value(call.head);
@@ -71,8 +71,8 @@ impl Command for Collect {
             redirect_env(engine_state, stack, &stack_captures);
             // for when we support `data | let x = $in;`
             // remove the variables added earlier
-            for var_id in capture_block.captures.keys() {
+            for (var_id, _) in capture_block.captures {
-                stack_captures.remove_var(*var_id);
+                stack_captures.remove_var(var_id);
             }
             if let Some(u) = saved_positional {
                 stack_captures.remove_var(u);

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

@@ -72,7 +72,7 @@ impl Command for Do {
         let capture_errors = call.has_flag("capture-errors");
         let has_env = call.has_flag("env");
 
-        let mut callee_stack = caller_stack.captures_to_stack(&block.captures);
+        let mut callee_stack = caller_stack.captures_to_stack(block.captures);
         let block = engine_state.get_block(block.block_id);
 
         let params: Vec<_> = block

crates/nu-color-config/src/style_computer.rs

@@ -61,7 +61,7 @@ impl<'a> StyleComputer<'a> {
                         let block = self.engine_state.get_block(val.block_id).clone();
                         // Because captures_to_stack() clones, we don't need to use with_env() here
                         // (contrast with_env() usage in `each` or `do`).
-                        let mut stack = self.stack.captures_to_stack(&val.captures);
+                        let mut stack = self.stack.captures_to_stack(val.captures.clone());
 
                         // Support 1-argument blocks as well as 0-argument blocks.
                         if let Some(var) = block.signature.get_positional(0) {

crates/nu-command/src/charting/hashable_value.rs

@@ -182,7 +182,7 @@ mod test {
         ast::{CellPath, PathMember},
         engine::Closure,
     };
-    use std::collections::{HashMap, HashSet};
+    use std::collections::HashSet;
 
     #[test]
     fn from_value() {
@@ -243,7 +243,7 @@ mod test {
             Value::closure(
                 Closure {
                     block_id: 0,
-                    captures: HashMap::new(),
+                    captures: Vec::new(),
                 },
                 span,
             ),

crates/nu-command/src/debug/explain.rs

@@ -41,7 +41,7 @@ impl Command for Explain {
         let capture_block: Closure = call.req(engine_state, stack, 0)?;
         let block = engine_state.get_block(capture_block.block_id);
         let ctrlc = engine_state.ctrlc.clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
 
         let elements = get_pipeline_elements(engine_state, &mut stack, block)?;
 

crates/nu-command/src/env/export_env.rs

@@ -37,7 +37,7 @@ impl Command for ExportEnv {
     ) -> Result<PipelineData, ShellError> {
         let capture_block: Closure = call.req(engine_state, caller_stack, 0)?;
         let block = engine_state.get_block(capture_block.block_id);
-        let mut callee_stack = caller_stack.captures_to_stack(&capture_block.captures);
+        let mut callee_stack = caller_stack.captures_to_stack(capture_block.captures);
 
         let _ = eval_block(
             engine_state,

crates/nu-command/src/env/with_env.rs

@@ -85,7 +85,7 @@ fn with_env(
 
     let capture_block: Closure = call.req(engine_state, stack, 1)?;
     let block = engine_state.get_block(capture_block.block_id);
-    let mut stack = stack.captures_to_stack(&capture_block.captures);
+    let mut stack = stack.captures_to_stack(capture_block.captures);
 
     let mut env: HashMap<String, Value> = HashMap::new();
 

crates/nu-command/src/filters/each.rs

@@ -121,7 +121,7 @@ with 'transpose' first."#
         let outer_ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(capture_block.block_id).clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let span = call.head;

crates/nu-command/src/filters/filter.rs

@@ -58,7 +58,7 @@ a variable. On the other hand, the "row condition" syntax is not supported."#
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(capture_block.block_id).clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let span = call.head;

crates/nu-command/src/filters/group_by.rs

@@ -246,7 +246,7 @@ fn group_closure(
 
     for value in values {
         if let Some(capture_block) = &block {
-            let mut stack = stack.captures_to_stack(&capture_block.captures);
+            let mut stack = stack.captures_to_stack(capture_block.captures.clone());
             let block = engine_state.get_block(capture_block.block_id);
             let pipeline = eval_block(
                 engine_state,

crates/nu-command/src/filters/insert.rs

@@ -125,7 +125,7 @@ fn insert(
         let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
 

crates/nu-command/src/filters/items.rs

@@ -49,7 +49,7 @@ impl Command for Items {
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(capture_block.block_id).clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let span = call.head;

crates/nu-command/src/filters/par_each.rs

@@ -128,7 +128,7 @@ impl Command for ParEach {
         let ctrlc = engine_state.ctrlc.clone();
         let outer_ctrlc = engine_state.ctrlc.clone();
         let block_id = capture_block.block_id;
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let span = call.head;
         let redirect_stdout = call.redirect_stdout;
         let redirect_stderr = call.redirect_stderr;

crates/nu-command/src/filters/reduce.rs

@@ -98,7 +98,7 @@ impl Command for Reduce {
 
         let fold: Option<Value> = call.get_flag(engine_state, stack, "fold")?;
         let capture_block: Closure = call.req(engine_state, stack, 0)?;
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let block = engine_state.get_block(capture_block.block_id);
         let ctrlc = engine_state.ctrlc.clone();
 

crates/nu-command/src/filters/rename.rs

@@ -141,7 +141,7 @@ fn rename(
         if let Some(capture_block) = call.get_flag::<Closure>(engine_state, stack, "block")? {
             let engine_state = engine_state.clone();
             let block = engine_state.get_block(capture_block.block_id).clone();
-            let stack = stack.captures_to_stack(&capture_block.captures);
+            let stack = stack.captures_to_stack(capture_block.captures);
             let orig_env_vars = stack.env_vars.clone();
             let orig_env_hidden = stack.env_hidden.clone();
             Some((engine_state, block, stack, orig_env_vars, orig_env_hidden))

crates/nu-command/src/filters/skip/skip_until.rs

@@ -86,7 +86,7 @@ impl Command for SkipUntil {
 
         let block = engine_state.get_block(capture_block.block_id).clone();
         let var_id = block.signature.get_positional(0).and_then(|arg| arg.var_id);
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
 
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();

crates/nu-command/src/filters/skip/skip_while.rs

@@ -91,7 +91,7 @@ impl Command for SkipWhile {
 
         let block = engine_state.get_block(capture_block.block_id).clone();
         let var_id = block.signature.get_positional(0).and_then(|arg| arg.var_id);
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
 
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();

crates/nu-command/src/filters/take/take_until.rs

@@ -82,7 +82,7 @@ impl Command for TakeUntil {
         let block = engine_state.get_block(capture_block.block_id).clone();
         let var_id = block.signature.get_positional(0).and_then(|arg| arg.var_id);
 
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
 
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();

crates/nu-command/src/filters/take/take_while.rs

@@ -82,7 +82,7 @@ impl Command for TakeWhile {
         let block = engine_state.get_block(capture_block.block_id).clone();
         let var_id = block.signature.get_positional(0).and_then(|arg| arg.var_id);
 
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
 
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();

crates/nu-command/src/filters/update.rs

@@ -122,7 +122,7 @@ fn update(
         let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
 

crates/nu-command/src/filters/upsert.rs

@@ -144,7 +144,7 @@ fn upsert(
         let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
+        let mut stack = stack.captures_to_stack(capture_block.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
 

crates/nu-command/src/filters/utils.rs

@@ -30,7 +30,7 @@ pub fn boolean_fold(
 
     let block = engine_state.get_block(block_id);
     let var_id = block.signature.get_positional(0).and_then(|arg| arg.var_id);
-    let mut stack = stack.captures_to_stack(&capture_block.captures);
+    let mut stack = stack.captures_to_stack(capture_block.captures);
 
     let orig_env_vars = stack.env_vars.clone();
     let orig_env_hidden = stack.env_hidden.clone();

crates/nu-command/src/filters/where_.rs

@@ -59,7 +59,7 @@ not supported."#
         let span = call.head;
 
         let metadata = input.metadata();
-        let mut stack = stack.captures_to_stack(&closure.captures);
+        let mut stack = stack.captures_to_stack(closure.captures);
         let block = engine_state.get_block(closure.block_id).clone();
 
         let orig_env_vars = stack.env_vars.clone();

crates/nu-command/src/generators/generate.rs

@@ -102,7 +102,7 @@ used as the next argument to the closure, otherwise generation stops.
         let block = engine_state.get_block(capture_block.item.block_id).clone();
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
-        let mut stack = stack.captures_to_stack(&capture_block.item.captures);
+        let mut stack = stack.captures_to_stack(capture_block.item.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let redirect_stdout = call.redirect_stdout;

crates/nu-command/src/generators/unfold.rs

@@ -114,7 +114,7 @@ used as the next argument to the closure, otherwise generation stops.
         let block = engine_state.get_block(capture_block.item.block_id).clone();
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
-        let mut stack = stack.captures_to_stack(&capture_block.item.captures);
+        let mut stack = stack.captures_to_stack(capture_block.item.captures);
         let orig_env_vars = stack.env_vars.clone();
         let orig_env_hidden = stack.env_hidden.clone();
         let redirect_stdout = call.redirect_stdout;

crates/nu-engine/src/eval.rs

@@ -524,19 +524,15 @@ pub fn eval_expression(
             )
         }
         Expr::RowCondition(block_id) | Expr::Closure(block_id) => {
-            let mut captures = HashMap::new();
+            let block_id = *block_id;
-            let block = engine_state.get_block(*block_id);
+            let captures = engine_state
+                .get_block(block_id)
+                .captures
+                .iter()
+                .map(|&id| stack.get_var(id, expr.span).map(|var| (id, var)))
+                .collect::<Result<_, _>>()?;
 
-            for var_id in &block.captures {
+            Ok(Value::closure(Closure { block_id, captures }, expr.span))
-                captures.insert(*var_id, stack.get_var(*var_id, expr.span)?);
-            }
-            Ok(Value::closure(
-                Closure {
-                    block_id: *block_id,
-                    captures,
-                },
-                expr.span,
-            ))
         }
         Expr::Block(block_id) => Ok(Value::block(*block_id, expr.span)),
         Expr::List(x) => {

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

@@ -1,5 +1,3 @@
-use std::collections::HashMap;
-
 use crate::{BlockId, Value, VarId};
 
 use serde::{Deserialize, Serialize};
@@ -7,7 +5,7 @@ use serde::{Deserialize, Serialize};
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Closure {
     pub block_id: BlockId,
-    pub captures: HashMap<VarId, Value>,
+    pub captures: Vec<(VarId, Value)>,
 }
 
 #[derive(Clone, Debug)]

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

@@ -138,19 +138,13 @@ impl Stack {
             })
     }
 
-    pub fn captures_to_stack(&self, captures: &HashMap<VarId, Value>) -> Stack {
+    pub fn captures_to_stack(&self, captures: Vec<(VarId, Value)>) -> Stack {
         // FIXME: this is probably slow
         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,
+            vars: captures,
             env_vars,
             env_hidden: self.env_hidden.clone(),
             active_overlays: self.active_overlays.clone(),

crates/nu-protocol/src/value/from_value.rs

@@ -1,4 +1,3 @@
-use std::collections::HashMap;
 use std::path::PathBuf;
 
 use crate::ast::{CellPath, MatchPattern, PathMember};
@@ -496,7 +495,7 @@ impl FromValue for Closure {
             Value::Closure { val, .. } => Ok(val),
             Value::Block { val, .. } => Ok(Closure {
                 block_id: val,
-                captures: HashMap::new(),
+                captures: Vec::new(),
             }),
             v => Err(ShellError::CantConvert {
                 to_type: "Closure".into(),