From 7884de194142d91f6823bbec4e09eba1bfb2a185 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 24 Feb 2024 20:58:01 +0100 Subject: [PATCH] Remove some unnecessary static `Vec`s (#11947) Avoid unnecessary allocations or larger iterator structs - Turn static `Vec`s into arrays when possible - Use `std::iter::once`/`empty` where applicable - Use `bool::then_some` in `detect column` `.chain` - Drop in the bucket: de-vec-ing tests --- .../src/completions/command_completions.rs | 4 +- crates/nu-cli/src/completions/completer.rs | 2 +- crates/nu-cli/src/reedline_config.rs | 2 +- crates/nu-cmd-lang/src/core_commands/do_.rs | 8 +- crates/nu-command/src/date/utils.rs | 2 +- crates/nu-command/src/filters/get.rs | 2 +- .../nu-command/src/strings/detect_columns.rs | 202 +++++++++--------- crates/nu-engine/src/eval.rs | 2 +- crates/nu-protocol/src/pipeline_data.rs | 2 +- .../src/playground/nu_process.rs | 2 +- 10 files changed, 115 insertions(+), 113 deletions(-) diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index 957cd9f84e..413eaaf5a5 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -253,7 +253,7 @@ mod command_completions_tests { #[test] fn test_find_non_whitespace_index() { - let commands = vec![ + let commands = [ (" hello", 4), ("sudo ", 0), (" sudo ", 2), @@ -273,7 +273,7 @@ mod command_completions_tests { #[test] fn test_is_last_command_passthrough() { - let commands = vec![ + let commands = [ (" hello", false), (" sudo ", true), ("sudo ", true), diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 4c18e6701a..5095f6bb4f 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -564,7 +564,7 @@ mod completer_tests { ); let mut completer = NuCompleter::new(engine_state.into(), Stack::new()); - let dataset = vec![ + let dataset = [ ("sudo", false, "", Vec::new()), ("sudo l", true, "l", vec!["ls", "let", "lines", "loop"]), (" sudo", false, "", Vec::new()), diff --git a/crates/nu-cli/src/reedline_config.rs b/crates/nu-cli/src/reedline_config.rs index adc50e9726..ed0a4d4943 100644 --- a/crates/nu-cli/src/reedline_config.rs +++ b/crates/nu-cli/src/reedline_config.rs @@ -84,7 +84,7 @@ pub(crate) fn add_menus( } // Checking if the default menus have been added from the config file - let default_menus = vec![ + let default_menus = [ ("completion_menu", DEFAULT_COMPLETION_MENU), ("history_menu", DEFAULT_HISTORY_MENU), ("help_menu", DEFAULT_HELP_MENU), diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index dc36a764d1..4931361a73 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -154,9 +154,9 @@ impl Command for Do { let ctrlc = stdout_stream.ctrlc.clone(); let span = stdout_stream.span; RawStream::new( - Box::new( - vec![stdout_stream.into_bytes().map(|s| s.item)].into_iter(), - ), + Box::new(std::iter::once( + stdout_stream.into_bytes().map(|s| s.item), + )), ctrlc, span, None, @@ -213,7 +213,7 @@ impl Command for Do { Ok(PipelineData::ExternalStream { stdout, stderr: Some(RawStream::new( - Box::new(vec![Ok(stderr_msg.into_bytes())].into_iter()), + Box::new(std::iter::once(Ok(stderr_msg.into_bytes()))), stderr_ctrlc, span, None, diff --git a/crates/nu-command/src/date/utils.rs b/crates/nu-command/src/date/utils.rs index c46f95faf9..f58b2a517c 100644 --- a/crates/nu-command/src/date/utils.rs +++ b/crates/nu-command/src/date/utils.rs @@ -46,7 +46,7 @@ pub(crate) fn generate_strftime_list(head: Span, show_parse_only_formats: bool) description: &'a str, } - let specifications = vec![ + let specifications = [ FormatSpecification { spec: "%Y", description: "The full proleptic Gregorian year, zero-padded to 4 digits.", diff --git a/crates/nu-command/src/filters/get.rs b/crates/nu-command/src/filters/get.rs index 1748b4cd43..9a8c2c5666 100644 --- a/crates/nu-command/src/filters/get.rs +++ b/crates/nu-command/src/filters/get.rs @@ -85,7 +85,7 @@ If multiple cell paths are given, this will produce a list of values."# } else { let mut output = vec![]; - let paths = vec![cell_path].into_iter().chain(rest); + let paths = std::iter::once(cell_path).chain(rest); let input = input.into_value(span); diff --git a/crates/nu-command/src/strings/detect_columns.rs b/crates/nu-command/src/strings/detect_columns.rs index cbaeb10174..ba09187f49 100644 --- a/crates/nu-command/src/strings/detect_columns.rs +++ b/crates/nu-command/src/strings/detect_columns.rs @@ -128,118 +128,120 @@ fn detect_columns( } } - Ok((if noheader { - vec![orig_headers].into_iter().chain(input) - } else { - vec![].into_iter().chain(input) - }) - .map(move |x| { - let row = find_columns(&x); + Ok(noheader + .then_some(orig_headers) + .into_iter() + .chain(input) + .map(move |x| { + let row = find_columns(&x); - let mut record = Record::new(); + let mut record = Record::new(); - if headers.len() == row.len() { - for (header, val) in headers.iter().zip(row.iter()) { - record.push(&header.item, Value::string(&val.item, name_span)); - } - } else { - let mut pre_output = vec![]; + if headers.len() == row.len() { + for (header, val) in headers.iter().zip(row.iter()) { + record.push(&header.item, Value::string(&val.item, name_span)); + } + } else { + let mut pre_output = vec![]; + + // column counts don't line up, so see if we can figure out why + for cell in row { + for header in &headers { + if cell.span.start <= header.span.end + && cell.span.end > header.span.start + { + pre_output.push(( + header.item.to_string(), + Value::string(&cell.item, name_span), + )); + } + } + } - // column counts don't line up, so see if we can figure out why - for cell in row { for header in &headers { - if cell.span.start <= header.span.end && cell.span.end > header.span.start { - pre_output.push(( - header.item.to_string(), - Value::string(&cell.item, name_span), - )); + let mut found = false; + for pre_o in &pre_output { + if pre_o.0 == header.item { + found = true; + break; + } + } + + if !found { + pre_output.push((header.item.to_string(), Value::nothing(name_span))); + } + } + + for header in &headers { + for pre_o in &pre_output { + if pre_o.0 == header.item { + record.push(&header.item, pre_o.1.clone()); + } } } } - for header in &headers { - let mut found = false; - for pre_o in &pre_output { - if pre_o.0 == header.item { - found = true; - break; + let (start_index, end_index) = if let Some(range) = &range { + match nu_cmd_base::util::process_range(range) { + Ok((l_idx, r_idx)) => { + let l_idx = if l_idx < 0 { + record.len() as isize + l_idx + } else { + l_idx + }; + + let r_idx = if r_idx < 0 { + record.len() as isize + r_idx + } else { + r_idx + }; + + if !(l_idx <= r_idx && (r_idx >= 0 || l_idx < (record.len() as isize))) + { + return Value::record(record, name_span); + } + + ( + l_idx.max(0) as usize, + (r_idx as usize + 1).min(record.len()), + ) + } + Err(processing_error) => { + let err = processing_error("could not find range index", name_span); + return Value::error(err, name_span); } } + } else { + return Value::record(record, name_span); + }; - if !found { - pre_output.push((header.item.to_string(), Value::nothing(name_span))); - } + let (mut cols, mut vals): (Vec<_>, Vec<_>) = record.into_iter().unzip(); + + // Merge Columns + ((start_index + 1)..(cols.len() - end_index + start_index + 1)).for_each(|idx| { + cols.swap(idx, end_index - start_index - 1 + idx); + }); + cols.truncate(cols.len() - end_index + start_index + 1); + + // Merge Values + let combined = vals + .iter() + .take(end_index) + .skip(start_index) + .map(|v| v.coerce_str().unwrap_or_default()) + .join(" "); + let binding = Value::string(combined, Span::unknown()); + let last_seg = vals.split_off(end_index); + vals.truncate(start_index); + vals.push(binding); + vals.extend(last_seg); + + match Record::from_raw_cols_vals(cols, vals, Span::unknown(), name_span) { + Ok(record) => Value::record(record, name_span), + Err(err) => Value::error(err, name_span), } - - for header in &headers { - for pre_o in &pre_output { - if pre_o.0 == header.item { - record.push(&header.item, pre_o.1.clone()); - } - } - } - } - - let (start_index, end_index) = if let Some(range) = &range { - match nu_cmd_base::util::process_range(range) { - Ok((l_idx, r_idx)) => { - let l_idx = if l_idx < 0 { - record.len() as isize + l_idx - } else { - l_idx - }; - - let r_idx = if r_idx < 0 { - record.len() as isize + r_idx - } else { - r_idx - }; - - if !(l_idx <= r_idx && (r_idx >= 0 || l_idx < (record.len() as isize))) { - return Value::record(record, name_span); - } - - ( - l_idx.max(0) as usize, - (r_idx as usize + 1).min(record.len()), - ) - } - Err(processing_error) => { - let err = processing_error("could not find range index", name_span); - return Value::error(err, name_span); - } - } - } else { - return Value::record(record, name_span); - }; - - let (mut cols, mut vals): (Vec<_>, Vec<_>) = record.into_iter().unzip(); - - // Merge Columns - ((start_index + 1)..(cols.len() - end_index + start_index + 1)).for_each(|idx| { - cols.swap(idx, end_index - start_index - 1 + idx); - }); - cols.truncate(cols.len() - end_index + start_index + 1); - - // Merge Values - let combined = vals - .iter() - .take(end_index) - .skip(start_index) - .map(|v| v.coerce_str().unwrap_or_default()) - .join(" "); - let binding = Value::string(combined, Span::unknown()); - let last_seg = vals.split_off(end_index); - vals.truncate(start_index); - vals.push(binding); - vals.extend(last_seg); - - match Record::from_raw_cols_vals(cols, vals, Span::unknown(), name_span) { - Ok(record) => Value::record(record, name_span), - Err(err) => Value::error(err, name_span), - } - }) - .into_pipeline_data(ctrlc)) + }) + .into_pipeline_data(ctrlc)) } else { Ok(PipelineData::empty()) } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index ccf49aaf58..ae18de9e6f 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -550,7 +550,7 @@ fn eval_element_with_input( // so nushell knows this result is not the last part of a command. ( Some(RawStream::new( - Box::new(vec![].into_iter()), + Box::new(std::iter::empty()), None, *span, Some(0), diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 21126e709a..b61973e016 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -605,7 +605,7 @@ impl PipelineData { .map(|bytes| bytes.item) .unwrap_or_default(); RawStream::new( - Box::new(vec![Ok(stderr_bytes)].into_iter()), + Box::new(std::iter::once(Ok(stderr_bytes))), stderr_ctrlc, stderr_span, None, diff --git a/crates/nu-test-support/src/playground/nu_process.rs b/crates/nu-test-support/src/playground/nu_process.rs index 401dbd4498..0031a173e8 100644 --- a/crates/nu-test-support/src/playground/nu_process.rs +++ b/crates/nu-test-support/src/playground/nu_process.rs @@ -82,7 +82,7 @@ impl NuProcess { command.env_clear(); - let paths = vec![test_bins_path()]; + let paths = [test_bins_path()]; let paths_joined = match std::env::join_paths(paths) { Ok(all) => all,