Use slices directly instead of &Vec (#10328)

Simplifies the signature, makes it more flexible.
Detected a few unnecessary allocations in the process.
This commit is contained in:
Stefan Holderbach 2023-09-12 05:38:20 +02:00 committed by GitHub
parent 84c10de864
commit e90b099622
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 48 additions and 46 deletions

View File

@ -263,7 +263,7 @@ mod command_completions_tests {
(" hello sud", 1), (" hello sud", 1),
]; ];
for (idx, ele) in commands.iter().enumerate() { 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); assert_eq!(index, ele.1, "Failed on index {}", idx);
} }
} }

View File

@ -144,7 +144,7 @@ impl Highlighter for NuHighlighter {
fn split_span_by_highlight_positions( fn split_span_by_highlight_positions(
line: &str, line: &str,
span: Span, span: Span,
highlight_positions: &Vec<usize>, highlight_positions: &[usize],
global_span_offset: usize, global_span_offset: usize,
) -> Vec<(Span, bool)> { ) -> Vec<(Span, bool)> {
let mut start = span.start; let mut start = span.start;

View File

@ -111,7 +111,7 @@ fn operate(
fn process_each_path( fn process_each_path(
mut value: Value, mut value: Value,
column_paths: &Vec<CellPath>, column_paths: &[CellPath],
text: &Option<String>, text: &Option<String>,
command_span: Span, command_span: Span,
) -> Value { ) -> Value {

View File

@ -10,7 +10,7 @@ pub fn check_example_input_and_output_types_match_command_signature(
example: &Example, example: &Example,
cwd: &std::path::Path, cwd: &std::path::Path,
engine_state: &mut Box<EngineState>, engine_state: &mut Box<EngineState>,
signature_input_output_types: &Vec<(Type, Type)>, signature_input_output_types: &[(Type, Type)],
signature_operates_on_cell_paths: bool, signature_operates_on_cell_paths: bool,
) -> HashSet<(Type, Type)> { ) -> HashSet<(Type, Type)> {
let mut witnessed_type_transformations = HashSet::<(Type, Type)>::new(); let mut witnessed_type_transformations = HashSet::<(Type, Type)>::new();

View File

@ -235,7 +235,7 @@ mod util {
} }
} }
fn convert_records_to_dataset(cols: &Vec<String>, records: Vec<Value>) -> Vec<Vec<String>> { fn convert_records_to_dataset(cols: &[String], records: Vec<Value>) -> Vec<Vec<String>> {
if !cols.is_empty() { if !cols.is_empty() {
create_table_for_record(cols, &records) create_table_for_record(cols, &records)
} else if cols.is_empty() && records.is_empty() { } else if cols.is_empty() && records.is_empty() {

View File

@ -294,7 +294,7 @@ fn record_matches_regex(values: &[Value], re: &Regex, config: &Config) -> bool {
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
fn highlight_terms_in_record_with_search_columns( fn highlight_terms_in_record_with_search_columns(
search_cols: &Vec<String>, search_cols: &[String],
record: &Record, record: &Record,
span: Span, span: Span,
config: &Config, config: &Config,
@ -507,7 +507,7 @@ fn value_should_be_printed(
filter_config: &Config, filter_config: &Config,
lower_terms: &[Value], lower_terms: &[Value],
span: Span, span: Span,
columns_to_search: &Vec<String>, columns_to_search: &[String],
invert: bool, invert: bool,
) -> bool { ) -> bool {
let lower_value = Value::string(value.into_string("", filter_config).to_lowercase(), span); 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( fn record_matches_term(
record: &Record, record: &Record,
columns_to_search: &Vec<String>, columns_to_search: &[String],
filter_config: &Config, filter_config: &Config,
term: &Value, term: &Value,
span: Span, span: Span,

View File

@ -210,7 +210,7 @@ pub fn group_no_grouper(values: Vec<Value>, span: Span) -> Result<Value, ShellEr
// TODO: refactor this, it's a bit of a mess // TODO: refactor this, it's a bit of a mess
fn group_closure( fn group_closure(
values: &Vec<Value>, values: &[Value],
span: Span, span: Span,
block: Option<Closure>, block: Option<Closure>,
stack: &mut Stack, stack: &mut Stack,
@ -219,7 +219,7 @@ fn group_closure(
) -> Result<Value, ShellError> { ) -> Result<Value, ShellError> {
let error_key = "error"; let error_key = "error";
let mut keys: Vec<Result<String, ShellError>> = vec![]; let mut keys: Vec<Result<String, ShellError>> = vec![];
let value_list = Value::list(values.clone(), span); let value_list = Value::list(values.to_vec(), span);
for value in values { for value in values {
if let Some(capture_block) = &block { if let Some(capture_block) = &block {

View File

@ -23,8 +23,6 @@ enum IncludeInner {
Yes, Yes,
} }
const EMPTY_COL_NAMES: &Vec<String> = &vec![];
impl Command for Join { impl Command for Join {
fn name(&self) -> &str { fn name(&self) -> &str {
"join" "join"
@ -142,8 +140,8 @@ fn join_type(call: &Call) -> Result<JoinType, nu_protocol::ShellError> {
} }
fn join( fn join(
left: &Vec<Value>, left: &[Value],
right: &Vec<Value>, right: &[Value],
left_join_key: &str, left_join_key: &str,
right_join_key: &str, right_join_key: &str,
join_type: JoinType, join_type: JoinType,
@ -248,7 +246,7 @@ fn join(
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
fn join_rows( fn join_rows(
result: &mut Vec<Value>, result: &mut Vec<Value>,
this: &Vec<Value>, this: &[Value],
this_join_key: &str, this_join_key: &str,
other: HashMap<String, Vec<&Record>>, other: HashMap<String, Vec<&Record>>,
other_keys: &[String], other_keys: &[String],
@ -323,20 +321,20 @@ fn join_rows(
// Return column names (i.e. ordered keys from the first row; we assume that // Return column names (i.e. ordered keys from the first row; we assume that
// these are the same for all rows). // these are the same for all rows).
fn column_names(table: &[Value]) -> &Vec<String> { fn column_names(table: &[Value]) -> &[String] {
table table
.iter() .iter()
.find_map(|val| match val { .find_map(|val| match val {
Value::Record { val, .. } => Some(&val.cols), Value::Record { val, .. } => Some(&*val.cols),
_ => None, _ => 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 // Create a map from value in `on` column to a list of the rows having that
// value. // value.
fn lookup_table<'a>( fn lookup_table<'a>(
rows: &'a Vec<Value>, rows: &'a [Value],
on: &str, on: &str,
sep: &str, sep: &str,
cap: usize, cap: usize,

View File

@ -76,7 +76,7 @@ impl Command for UniqBy {
let metadata = input.metadata(); let metadata = input.metadata();
let vec: Vec<_> = input.into_iter().collect(); let vec: Vec<_> = input.into_iter().collect();
match validate(vec.clone(), &columns, call.head) { match validate(&vec, &columns, call.head) {
Ok(_) => {} Ok(_) => {}
Err(err) => { Err(err) => {
return Err(err); return Err(err);
@ -113,7 +113,7 @@ impl Command for UniqBy {
} }
} }
fn validate(vec: Vec<Value>, columns: &Vec<String>, span: Span) -> Result<(), ShellError> { fn validate(vec: &[Value], columns: &[String], span: Span) -> Result<(), ShellError> {
let first = vec.first(); let first = vec.first();
if let Some(v) = first { if let Some(v) = first {
let val_span = v.span(); let val_span = v.span();
@ -129,7 +129,7 @@ fn validate(vec: Vec<Value>, columns: &Vec<String>, 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 { return Err(ShellError::CantFindColumn {
col_name: nonexistent, col_name: nonexistent,
span, span,

View File

@ -46,7 +46,7 @@ fn record_to_delimited(
} }
fn table_to_delimited( fn table_to_delimited(
vals: &Vec<Value>, vals: &[Value],
span: Span, span: Span,
separator: char, separator: char,
config: &Config, config: &Config,

View File

@ -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 { return Err(ShellError::CantFindColumn {
col_name: nonexistent, col_name: nonexistent,
span, span,

View File

@ -19,10 +19,10 @@ pub fn get_columns(input: &[Value]) -> Vec<String> {
} }
// If a column doesn't exist in the input, return it. // If a column doesn't exist in the input, return it.
pub fn nonexistent_column(inputs: Vec<String>, columns: Vec<String>) -> Option<String> { pub fn nonexistent_column(inputs: &[String], columns: &[String]) -> Option<String> {
let set: HashSet<String> = HashSet::from_iter(columns.iter().cloned()); let set: HashSet<String> = HashSet::from_iter(columns.iter().cloned());
for input in &inputs { for input in inputs {
if set.contains(input) { if set.contains(input) {
continue; continue;
} }

View File

@ -122,7 +122,7 @@ pub fn collect_input(value: Value) -> (Vec<String>, Vec<Vec<Value>>) {
} }
} }
fn convert_records_to_dataset(cols: &Vec<String>, records: Vec<Value>) -> Vec<Vec<Value>> { fn convert_records_to_dataset(cols: &[String], records: Vec<Value>) -> Vec<Vec<Value>> {
if !cols.is_empty() { if !cols.is_empty() {
create_table_for_record(cols, &records) create_table_for_record(cols, &records)
} else if cols.is_empty() && records.is_empty() { } else if cols.is_empty() && records.is_empty() {

View File

@ -182,7 +182,7 @@ impl Value {
/// If the `Value` is an Array, returns the associated vector. /// If the `Value` is an Array, returns the associated vector.
/// Returns None otherwise. /// Returns None otherwise.
pub fn as_array(&self) -> Option<&Vec<Value>> { pub fn as_array(&self) -> Option<&[Value]> {
match self { match self {
Value::Array(array) => Some(array), Value::Array(array) => Some(array),
_ => None, _ => None,

View File

@ -2887,7 +2887,7 @@ pub fn parse_overlay_hide(working_set: &mut StateWorkingSet, call: Box<Call>) ->
if !working_set if !working_set
.unique_overlay_names() .unique_overlay_names()
.contains(&overlay_name.as_bytes().to_vec()) .contains(&overlay_name.as_bytes())
{ {
working_set.error(ParseError::ActiveOverlayNotFound(overlay_name_span)); working_set.error(ParseError::ActiveOverlayNotFound(overlay_name_span));
return pipeline; return pipeline;

View File

@ -343,10 +343,11 @@ impl EngineState {
where where
'b: 'a, 'b: 'a,
{ {
self.scope self.scope.active_overlays.iter().filter(|id| {
.active_overlays !removed_overlays
.iter() .iter()
.filter(|id| !removed_overlays.contains(self.get_overlay_name(**id))) .any(|name| name == self.get_overlay_name(**id))
})
} }
pub fn active_overlays<'a, 'b>( pub fn active_overlays<'a, 'b>(
@ -363,7 +364,7 @@ impl EngineState {
pub fn active_overlay_names<'a, 'b>( pub fn active_overlay_names<'a, 'b>(
&'b self, &'b self,
removed_overlays: &'a [Vec<u8>], removed_overlays: &'a [Vec<u8>],
) -> impl DoubleEndedIterator<Item = &Vec<u8>> + 'a ) -> impl DoubleEndedIterator<Item = &[u8]> + 'a
where where
'b: 'a, 'b: 'a,
{ {
@ -389,7 +390,7 @@ impl EngineState {
.collect() .collect()
} }
pub fn last_overlay_name(&self, removed_overlays: &[Vec<u8>]) -> &Vec<u8> { pub fn last_overlay_name(&self, removed_overlays: &[Vec<u8>]) -> &[u8] {
self.active_overlay_names(removed_overlays) self.active_overlay_names(removed_overlays)
.last() .last()
.expect("internal error: no active overlays") .expect("internal error: no active overlays")
@ -402,7 +403,7 @@ impl EngineState {
.expect("internal error: no active overlays") .expect("internal error: no active overlays")
} }
pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &Vec<u8> { pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] {
&self &self
.scope .scope
.overlays .overlays
@ -931,7 +932,7 @@ impl EngineState {
.unwrap_or_default() .unwrap_or_default()
} }
pub fn get_file_contents(&self) -> &Vec<(Vec<u8>, usize, usize)> { pub fn get_file_contents(&self) -> &[(Vec<u8>, usize, usize)] {
&self.file_contents &self.file_contents
} }
@ -1081,7 +1082,7 @@ impl StateDelta {
self.scope.pop(); self.scope.pop();
} }
pub fn get_file_contents(&self) -> &Vec<(Vec<u8>, usize, usize)> { pub fn get_file_contents(&self) -> &[(Vec<u8>, usize, usize)] {
&self.file_contents &self.file_contents
} }
} }
@ -1127,8 +1128,8 @@ impl<'a> StateWorkingSet<'a> {
self.delta.num_modules() + self.permanent_state.num_modules() self.delta.num_modules() + self.permanent_state.num_modules()
} }
pub fn unique_overlay_names(&self) -> HashSet<&Vec<u8>> { pub fn unique_overlay_names(&self) -> HashSet<&[u8]> {
let mut names: HashSet<&Vec<u8>> = self.permanent_state.active_overlay_names(&[]).collect(); let mut names: HashSet<&[u8]> = self.permanent_state.active_overlay_names(&[]).collect();
for scope_frame in self.delta.scope.iter().rev() { for scope_frame in self.delta.scope.iter().rev() {
for overlay_id in scope_frame.active_overlays.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"); .expect("internal error: missing overlay");
names.insert(overlay_name); 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)) .map(|id| self.permanent_state.get_overlay(id))
} }
pub fn last_overlay_name(&self) -> &Vec<u8> { pub fn last_overlay_name(&self) -> &[u8] {
let mut removed_overlays = vec![]; let mut removed_overlays = vec![];
for scope_frame in self.delta.scope.iter().rev() { for scope_frame in self.delta.scope.iter().rev() {

View File

@ -108,7 +108,11 @@ impl ScopeFrame {
self.active_overlays self.active_overlays
.iter() .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() .copied()
.collect() .collect()
} }
@ -125,14 +129,14 @@ impl ScopeFrame {
.map(|id| self.get_overlay(id)) .map(|id| self.get_overlay(id))
} }
pub fn active_overlay_names(&self, removed_overlays: &mut Vec<Vec<u8>>) -> Vec<&Vec<u8>> { pub fn active_overlay_names(&self, removed_overlays: &mut Vec<Vec<u8>>) -> Vec<&[u8]> {
self.active_overlay_ids(removed_overlays) self.active_overlay_ids(removed_overlays)
.iter() .iter()
.map(|id| self.get_overlay_name(*id)) .map(|id| self.get_overlay_name(*id))
.collect() .collect()
} }
pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &Vec<u8> { pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] {
&self &self
.overlays .overlays
.get(overlay_id) .get(overlay_id)