diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index af935c7992..096ed1b5f7 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -124,7 +124,7 @@ impl NuCompleter { let output = parse(&mut working_set, Some("completer"), line.as_bytes(), false); - for pipeline in output.pipelines.iter() { + for pipeline in &output.pipelines { for pipeline_element in &pipeline.elements { let flattened = flatten_pipeline_element(&working_set, pipeline_element); let mut spans: Vec = vec![]; diff --git a/crates/nu-explore/src/nu_common/command.rs b/crates/nu-explore/src/nu_common/command.rs index e1a6e78774..bffc7b256b 100644 --- a/crates/nu-explore/src/nu_common/command.rs +++ b/crates/nu-explore/src/nu_common/command.rs @@ -91,6 +91,8 @@ fn eval_source2( // So we LITERALLY ignore all expressions except the LAST. if block.len() > 1 { let range = ..block.pipelines.len() - 1; + // Note: `make_mut` will mutate `&mut block: &mut Arc` + // for the outer fn scope `eval_block` Arc::make_mut(&mut block).pipelines.drain(range); } diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index d445b27872..ca70e79ff9 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; -use super::{Argument, Expr, ExternalArgument, RecordItem}; +use super::{Argument, Block, Expr, ExternalArgument, RecordItem}; use crate::ast::ImportPattern; use crate::DeclId; use crate::{engine::StateWorkingSet, BlockId, Signature, Span, Type, VarId, IN_VARIABLE_ID}; @@ -327,7 +327,8 @@ impl Expression { expr.replace_span(working_set, replaced, new_span); } Expr::Block(block_id) => { - let mut block = (**working_set.get_block(*block_id)).clone(); + // We are cloning the Block itself, rather than the Arc around it. + let mut block = Block::clone(working_set.get_block(*block_id)); for pipeline in block.pipelines.iter_mut() { for element in pipeline.elements.iter_mut() { diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index eb2eb69492..c55ef0fd65 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -79,8 +79,11 @@ pub struct EngineState { pub(super) virtual_paths: Vec<(String, VirtualPath)>, vars: Vec, decls: Arc>>, - pub(super) blocks: Vec>, - pub(super) modules: Vec>, + // The Vec is wrapped in Arc so that if we don't need to modify the list, we can just clone + // the reference and not have to clone each individual Arc inside. These lists can be + // especially long, so it helps + pub(super) blocks: Arc>>, + pub(super) modules: Arc>>, usage: Usage, pub scope: ScopeFrame, pub ctrlc: Option>, @@ -128,10 +131,10 @@ impl EngineState { Variable::new(Span::new(0, 0), Type::Any, false), ], decls: Arc::new(vec![]), - blocks: vec![], - modules: vec![Arc::new(Module::new( + blocks: Arc::new(vec![]), + modules: Arc::new(vec![Arc::new(Module::new( DEFAULT_OVERLAY_NAME.as_bytes().to_vec(), - ))], + ))]), usage: Usage::new(), // make sure we have some default overlay: scope: ScopeFrame::with_empty_overlay( @@ -184,14 +187,18 @@ impl EngineState { self.files.extend(delta.files); self.virtual_paths.extend(delta.virtual_paths); self.vars.extend(delta.vars); - self.blocks.extend(delta.blocks); - self.modules.extend(delta.modules); self.usage.merge_with(delta.usage); - // Avoid potentially cloning the Arc if we aren't adding anything + // Avoid potentially cloning the Arcs if we aren't adding anything if !delta.decls.is_empty() { Arc::make_mut(&mut self.decls).extend(delta.decls); } + if !delta.blocks.is_empty() { + Arc::make_mut(&mut self.blocks).extend(delta.blocks); + } + if !delta.modules.is_empty() { + Arc::make_mut(&mut self.modules).extend(delta.modules); + } let first = delta.scope.remove(0); diff --git a/crates/nu-protocol/src/engine/state_delta.rs b/crates/nu-protocol/src/engine/state_delta.rs index 23309c78f8..c7df18c890 100644 --- a/crates/nu-protocol/src/engine/state_delta.rs +++ b/crates/nu-protocol/src/engine/state_delta.rs @@ -3,7 +3,6 @@ use super::{usage::Usage, Command, EngineState, OverlayFrame, ScopeFrame, Variab use crate::ast::Block; use crate::Module; -#[cfg(feature = "plugin")] use std::sync::Arc; #[cfg(feature = "plugin")] diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index be8368de5f..29e27c6cc5 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -10,7 +10,6 @@ use core::panic; use std::collections::{HashMap, HashSet}; use std::path::PathBuf; -#[cfg(feature = "plugin")] use std::sync::Arc; #[cfg(feature = "plugin")] @@ -960,7 +959,7 @@ impl<'a> StateWorkingSet<'a> { } } - for block in &self.permanent_state.blocks { + for block in self.permanent_state.blocks.iter() { if Some(span) == block.span { return Some(block.clone()); }