From e90b099622907fe34a1081ba547ef85e11692f64 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Tue, 12 Sep 2023 05:38:20 +0200 Subject: [PATCH] Use slices directly instead of `&Vec` (#10328) Simplifies the signature, makes it more flexible. Detected a few unnecessary allocations in the process. --- .../src/completions/command_completions.rs | 2 +- crates/nu-cli/src/syntax_highlight.rs | 2 +- .../src/extra/platform/ansi/link.rs | 2 +- crates/nu-cmd-lang/src/example_support.rs | 2 +- crates/nu-command/src/debug/inspect_table.rs | 2 +- crates/nu-command/src/filters/find.rs | 6 ++--- crates/nu-command/src/filters/group_by.rs | 4 +-- crates/nu-command/src/filters/join.rs | 16 +++++------ crates/nu-command/src/filters/uniq_by.rs | 6 ++--- crates/nu-command/src/formats/to/delimited.rs | 2 +- crates/nu-command/src/sort_utils.rs | 3 +-- crates/nu-engine/src/column.rs | 4 +-- crates/nu-explore/src/nu_common/value.rs | 2 +- crates/nu-json/src/value.rs | 2 +- crates/nu-parser/src/parse_keywords.rs | 2 +- crates/nu-protocol/src/engine/engine_state.rs | 27 ++++++++++--------- crates/nu-protocol/src/engine/overlay.rs | 10 ++++--- 17 files changed, 48 insertions(+), 46 deletions(-) diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index 8687c9523739..c99c00985e5a 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -263,7 +263,7 @@ mod command_completions_tests { (" hello sud", 1), ]; for (idx, ele) in commands.iter().enumerate() { - let index = find_non_whitespace_index(&Vec::from(ele.0.as_bytes()), 0); + let index = find_non_whitespace_index(ele.0.as_bytes(), 0); assert_eq!(index, ele.1, "Failed on index {}", idx); } } diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index d48c29c08ca2..09c9dd476c7a 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -144,7 +144,7 @@ impl Highlighter for NuHighlighter { fn split_span_by_highlight_positions( line: &str, span: Span, - highlight_positions: &Vec, + highlight_positions: &[usize], global_span_offset: usize, ) -> Vec<(Span, bool)> { let mut start = span.start; diff --git a/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs b/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs index f2db38e8ec8d..037e4e082c24 100644 --- a/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs +++ b/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs @@ -111,7 +111,7 @@ fn operate( fn process_each_path( mut value: Value, - column_paths: &Vec, + column_paths: &[CellPath], text: &Option, command_span: Span, ) -> Value { diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 5076ad6bbccb..db34962f8069 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -10,7 +10,7 @@ pub fn check_example_input_and_output_types_match_command_signature( example: &Example, cwd: &std::path::Path, engine_state: &mut Box, - signature_input_output_types: &Vec<(Type, Type)>, + signature_input_output_types: &[(Type, Type)], signature_operates_on_cell_paths: bool, ) -> HashSet<(Type, Type)> { let mut witnessed_type_transformations = HashSet::<(Type, Type)>::new(); diff --git a/crates/nu-command/src/debug/inspect_table.rs b/crates/nu-command/src/debug/inspect_table.rs index 58fdad90b835..432ac47e53ad 100644 --- a/crates/nu-command/src/debug/inspect_table.rs +++ b/crates/nu-command/src/debug/inspect_table.rs @@ -235,7 +235,7 @@ mod util { } } - fn convert_records_to_dataset(cols: &Vec, records: Vec) -> Vec> { + fn convert_records_to_dataset(cols: &[String], records: Vec) -> Vec> { if !cols.is_empty() { create_table_for_record(cols, &records) } else if cols.is_empty() && records.is_empty() { diff --git a/crates/nu-command/src/filters/find.rs b/crates/nu-command/src/filters/find.rs index 9cf4c4ab0b63..0afc1a2950c9 100644 --- a/crates/nu-command/src/filters/find.rs +++ b/crates/nu-command/src/filters/find.rs @@ -294,7 +294,7 @@ fn record_matches_regex(values: &[Value], re: &Regex, config: &Config) -> bool { #[allow(clippy::too_many_arguments)] fn highlight_terms_in_record_with_search_columns( - search_cols: &Vec, + search_cols: &[String], record: &Record, span: Span, config: &Config, @@ -507,7 +507,7 @@ fn value_should_be_printed( filter_config: &Config, lower_terms: &[Value], span: Span, - columns_to_search: &Vec, + columns_to_search: &[String], invert: bool, ) -> bool { let lower_value = Value::string(value.into_string("", filter_config).to_lowercase(), span); @@ -561,7 +561,7 @@ fn term_equals_value(term: &Value, value: &Value, span: Span) -> bool { fn record_matches_term( record: &Record, - columns_to_search: &Vec, + columns_to_search: &[String], filter_config: &Config, term: &Value, span: Span, diff --git a/crates/nu-command/src/filters/group_by.rs b/crates/nu-command/src/filters/group_by.rs index bc4383dbd382..3f4c8417f483 100644 --- a/crates/nu-command/src/filters/group_by.rs +++ b/crates/nu-command/src/filters/group_by.rs @@ -210,7 +210,7 @@ pub fn group_no_grouper(values: Vec, span: Span) -> Result, + values: &[Value], span: Span, block: Option, stack: &mut Stack, @@ -219,7 +219,7 @@ fn group_closure( ) -> Result { let error_key = "error"; let mut keys: Vec> = vec![]; - let value_list = Value::list(values.clone(), span); + let value_list = Value::list(values.to_vec(), span); for value in values { if let Some(capture_block) = &block { diff --git a/crates/nu-command/src/filters/join.rs b/crates/nu-command/src/filters/join.rs index d278f31d40e1..f02f738ff380 100644 --- a/crates/nu-command/src/filters/join.rs +++ b/crates/nu-command/src/filters/join.rs @@ -23,8 +23,6 @@ enum IncludeInner { Yes, } -const EMPTY_COL_NAMES: &Vec = &vec![]; - impl Command for Join { fn name(&self) -> &str { "join" @@ -142,8 +140,8 @@ fn join_type(call: &Call) -> Result { } fn join( - left: &Vec, - right: &Vec, + left: &[Value], + right: &[Value], left_join_key: &str, right_join_key: &str, join_type: JoinType, @@ -248,7 +246,7 @@ fn join( #[allow(clippy::too_many_arguments)] fn join_rows( result: &mut Vec, - this: &Vec, + this: &[Value], this_join_key: &str, other: HashMap>, other_keys: &[String], @@ -323,20 +321,20 @@ fn join_rows( // Return column names (i.e. ordered keys from the first row; we assume that // these are the same for all rows). -fn column_names(table: &[Value]) -> &Vec { +fn column_names(table: &[Value]) -> &[String] { table .iter() .find_map(|val| match val { - Value::Record { val, .. } => Some(&val.cols), + Value::Record { val, .. } => Some(&*val.cols), _ => None, }) - .unwrap_or(EMPTY_COL_NAMES) + .unwrap_or_default() } // Create a map from value in `on` column to a list of the rows having that // value. fn lookup_table<'a>( - rows: &'a Vec, + rows: &'a [Value], on: &str, sep: &str, cap: usize, diff --git a/crates/nu-command/src/filters/uniq_by.rs b/crates/nu-command/src/filters/uniq_by.rs index e5d6c36ada3e..bad32073a0a9 100644 --- a/crates/nu-command/src/filters/uniq_by.rs +++ b/crates/nu-command/src/filters/uniq_by.rs @@ -76,7 +76,7 @@ impl Command for UniqBy { let metadata = input.metadata(); let vec: Vec<_> = input.into_iter().collect(); - match validate(vec.clone(), &columns, call.head) { + match validate(&vec, &columns, call.head) { Ok(_) => {} Err(err) => { return Err(err); @@ -113,7 +113,7 @@ impl Command for UniqBy { } } -fn validate(vec: Vec, columns: &Vec, span: Span) -> Result<(), ShellError> { +fn validate(vec: &[Value], columns: &[String], span: Span) -> Result<(), ShellError> { let first = vec.first(); if let Some(v) = first { let val_span = v.span(); @@ -129,7 +129,7 @@ fn validate(vec: Vec, columns: &Vec, span: Span) -> Result<(), Sh )); } - if let Some(nonexistent) = nonexistent_column(columns.clone(), record.cols.clone()) { + if let Some(nonexistent) = nonexistent_column(columns, &record.cols) { return Err(ShellError::CantFindColumn { col_name: nonexistent, span, diff --git a/crates/nu-command/src/formats/to/delimited.rs b/crates/nu-command/src/formats/to/delimited.rs index 7526307620fa..6036a82a8dd3 100644 --- a/crates/nu-command/src/formats/to/delimited.rs +++ b/crates/nu-command/src/formats/to/delimited.rs @@ -46,7 +46,7 @@ fn record_to_delimited( } fn table_to_delimited( - vals: &Vec, + vals: &[Value], span: Span, separator: char, config: &Config, diff --git a/crates/nu-command/src/sort_utils.rs b/crates/nu-command/src/sort_utils.rs index 2f7874e449c2..47db220b97cc 100644 --- a/crates/nu-command/src/sort_utils.rs +++ b/crates/nu-command/src/sort_utils.rs @@ -77,8 +77,7 @@ pub fn sort( )); } - if let Some(nonexistent) = nonexistent_column(sort_columns.clone(), record.cols.clone()) - { + if let Some(nonexistent) = nonexistent_column(&sort_columns, &record.cols) { return Err(ShellError::CantFindColumn { col_name: nonexistent, span, diff --git a/crates/nu-engine/src/column.rs b/crates/nu-engine/src/column.rs index 879a3ced8ffe..23108d517eaf 100644 --- a/crates/nu-engine/src/column.rs +++ b/crates/nu-engine/src/column.rs @@ -19,10 +19,10 @@ pub fn get_columns(input: &[Value]) -> Vec { } // If a column doesn't exist in the input, return it. -pub fn nonexistent_column(inputs: Vec, columns: Vec) -> Option { +pub fn nonexistent_column(inputs: &[String], columns: &[String]) -> Option { let set: HashSet = HashSet::from_iter(columns.iter().cloned()); - for input in &inputs { + for input in inputs { if set.contains(input) { continue; } diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index 86f8896e6471..cecab073d603 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -122,7 +122,7 @@ pub fn collect_input(value: Value) -> (Vec, Vec>) { } } -fn convert_records_to_dataset(cols: &Vec, records: Vec) -> Vec> { +fn convert_records_to_dataset(cols: &[String], records: Vec) -> Vec> { if !cols.is_empty() { create_table_for_record(cols, &records) } else if cols.is_empty() && records.is_empty() { diff --git a/crates/nu-json/src/value.rs b/crates/nu-json/src/value.rs index daf09595dc8c..1ead5756e20c 100644 --- a/crates/nu-json/src/value.rs +++ b/crates/nu-json/src/value.rs @@ -182,7 +182,7 @@ impl Value { /// If the `Value` is an Array, returns the associated vector. /// Returns None otherwise. - pub fn as_array(&self) -> Option<&Vec> { + pub fn as_array(&self) -> Option<&[Value]> { match self { Value::Array(array) => Some(array), _ => None, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 253c965df709..1389720f364a 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -2887,7 +2887,7 @@ pub fn parse_overlay_hide(working_set: &mut StateWorkingSet, call: Box) -> if !working_set .unique_overlay_names() - .contains(&overlay_name.as_bytes().to_vec()) + .contains(&overlay_name.as_bytes()) { working_set.error(ParseError::ActiveOverlayNotFound(overlay_name_span)); return pipeline; diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index d92bfcb0a91c..0ebde03162c8 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -343,10 +343,11 @@ impl EngineState { where 'b: 'a, { - self.scope - .active_overlays - .iter() - .filter(|id| !removed_overlays.contains(self.get_overlay_name(**id))) + self.scope.active_overlays.iter().filter(|id| { + !removed_overlays + .iter() + .any(|name| name == self.get_overlay_name(**id)) + }) } pub fn active_overlays<'a, 'b>( @@ -363,7 +364,7 @@ impl EngineState { pub fn active_overlay_names<'a, 'b>( &'b self, removed_overlays: &'a [Vec], - ) -> impl DoubleEndedIterator> + 'a + ) -> impl DoubleEndedIterator + 'a where 'b: 'a, { @@ -389,7 +390,7 @@ impl EngineState { .collect() } - pub fn last_overlay_name(&self, removed_overlays: &[Vec]) -> &Vec { + pub fn last_overlay_name(&self, removed_overlays: &[Vec]) -> &[u8] { self.active_overlay_names(removed_overlays) .last() .expect("internal error: no active overlays") @@ -402,7 +403,7 @@ impl EngineState { .expect("internal error: no active overlays") } - pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &Vec { + pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] { &self .scope .overlays @@ -931,7 +932,7 @@ impl EngineState { .unwrap_or_default() } - pub fn get_file_contents(&self) -> &Vec<(Vec, usize, usize)> { + pub fn get_file_contents(&self) -> &[(Vec, usize, usize)] { &self.file_contents } @@ -1081,7 +1082,7 @@ impl StateDelta { self.scope.pop(); } - pub fn get_file_contents(&self) -> &Vec<(Vec, usize, usize)> { + pub fn get_file_contents(&self) -> &[(Vec, usize, usize)] { &self.file_contents } } @@ -1127,8 +1128,8 @@ impl<'a> StateWorkingSet<'a> { self.delta.num_modules() + self.permanent_state.num_modules() } - pub fn unique_overlay_names(&self) -> HashSet<&Vec> { - let mut names: HashSet<&Vec> = self.permanent_state.active_overlay_names(&[]).collect(); + pub fn unique_overlay_names(&self) -> HashSet<&[u8]> { + let mut names: HashSet<&[u8]> = self.permanent_state.active_overlay_names(&[]).collect(); for scope_frame in self.delta.scope.iter().rev() { for overlay_id in scope_frame.active_overlays.iter().rev() { @@ -1138,7 +1139,7 @@ impl<'a> StateWorkingSet<'a> { .expect("internal error: missing overlay"); names.insert(overlay_name); - names.retain(|n| !scope_frame.removed_overlays.contains(n)); + names.retain(|n| !scope_frame.removed_overlays.iter().any(|m| n == m)); } } @@ -1820,7 +1821,7 @@ impl<'a> StateWorkingSet<'a> { .map(|id| self.permanent_state.get_overlay(id)) } - pub fn last_overlay_name(&self) -> &Vec { + pub fn last_overlay_name(&self) -> &[u8] { let mut removed_overlays = vec![]; for scope_frame in self.delta.scope.iter().rev() { diff --git a/crates/nu-protocol/src/engine/overlay.rs b/crates/nu-protocol/src/engine/overlay.rs index d28a90479fb6..96573a7f5083 100644 --- a/crates/nu-protocol/src/engine/overlay.rs +++ b/crates/nu-protocol/src/engine/overlay.rs @@ -108,7 +108,11 @@ impl ScopeFrame { self.active_overlays .iter() - .filter(|id| !removed_overlays.contains(self.get_overlay_name(**id))) + .filter(|id| { + !removed_overlays + .iter() + .any(|name| name == self.get_overlay_name(**id)) + }) .copied() .collect() } @@ -125,14 +129,14 @@ impl ScopeFrame { .map(|id| self.get_overlay(id)) } - pub fn active_overlay_names(&self, removed_overlays: &mut Vec>) -> Vec<&Vec> { + pub fn active_overlay_names(&self, removed_overlays: &mut Vec>) -> Vec<&[u8]> { self.active_overlay_ids(removed_overlays) .iter() .map(|id| self.get_overlay_name(*id)) .collect() } - pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &Vec { + pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] { &self .overlays .get(overlay_id)