Skip to content

Commit

Permalink
Use slices directly instead of &Vec (#10328)
Browse files Browse the repository at this point in the history
Simplifies the signature, makes it more flexible.
Detected a few unnecessary allocations in the process.
  • Loading branch information
sholderbach committed Sep 12, 2023
1 parent 84c10de commit e90b099
Show file tree
Hide file tree
Showing 17 changed files with 48 additions and 46 deletions.
2 changes: 1 addition & 1 deletion crates/nu-cli/src/completions/command_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/syntax_highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl Highlighter for NuHighlighter {
fn split_span_by_highlight_positions(
line: &str,
span: Span,
highlight_positions: &Vec<usize>,
highlight_positions: &[usize],
global_span_offset: usize,
) -> Vec<(Span, bool)> {
let mut start = span.start;
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cmd-extra/src/extra/platform/ansi/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ fn operate(

fn process_each_path(
mut value: Value,
column_paths: &Vec<CellPath>,
column_paths: &[CellPath],
text: &Option<String>,
command_span: Span,
) -> Value {
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cmd-lang/src/example_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<EngineState>,
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();
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/debug/inspect_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
create_table_for_record(cols, &records)
} else if cols.is_empty() && records.is_empty() {
Expand Down
6 changes: 3 additions & 3 deletions crates/nu-command/src/filters/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
search_cols: &[String],
record: &Record,
span: Span,
config: &Config,
Expand Down Expand Up @@ -507,7 +507,7 @@ fn value_should_be_printed(
filter_config: &Config,
lower_terms: &[Value],
span: Span,
columns_to_search: &Vec<String>,
columns_to_search: &[String],
invert: bool,
) -> bool {
let lower_value = Value::string(value.into_string("", filter_config).to_lowercase(), span);
Expand Down Expand Up @@ -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<String>,
columns_to_search: &[String],
filter_config: &Config,
term: &Value,
span: Span,
Expand Down
4 changes: 2 additions & 2 deletions crates/nu-command/src/filters/group_by.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
fn group_closure(
values: &Vec<Value>,
values: &[Value],
span: Span,
block: Option<Closure>,
stack: &mut Stack,
Expand All @@ -219,7 +219,7 @@ fn group_closure(
) -> Result<Value, ShellError> {
let error_key = "error";
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 {
if let Some(capture_block) = &block {
Expand Down
16 changes: 7 additions & 9 deletions crates/nu-command/src/filters/join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ enum IncludeInner {
Yes,
}

const EMPTY_COL_NAMES: &Vec<String> = &vec![];

impl Command for Join {
fn name(&self) -> &str {
"join"
Expand Down Expand Up @@ -142,8 +140,8 @@ fn join_type(call: &Call) -> Result<JoinType, nu_protocol::ShellError> {
}

fn join(
left: &Vec<Value>,
right: &Vec<Value>,
left: &[Value],
right: &[Value],
left_join_key: &str,
right_join_key: &str,
join_type: JoinType,
Expand Down Expand Up @@ -248,7 +246,7 @@ fn join(
#[allow(clippy::too_many_arguments)]
fn join_rows(
result: &mut Vec<Value>,
this: &Vec<Value>,
this: &[Value],
this_join_key: &str,
other: HashMap<String, Vec<&Record>>,
other_keys: &[String],
Expand Down Expand Up @@ -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<String> {
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<Value>,
rows: &'a [Value],
on: &str,
sep: &str,
cap: usize,
Expand Down
6 changes: 3 additions & 3 deletions crates/nu-command/src/filters/uniq_by.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
if let Some(v) = first {
let val_span = v.span();
Expand All @@ -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 {
col_name: nonexistent,
span,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/formats/to/delimited.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn record_to_delimited(
}

fn table_to_delimited(
vals: &Vec<Value>,
vals: &[Value],
span: Span,
separator: char,
config: &Config,
Expand Down
3 changes: 1 addition & 2 deletions crates/nu-command/src/sort_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/nu-engine/src/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ pub fn get_columns(input: &[Value]) -> Vec<String> {
}

// 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());

for input in &inputs {
for input in inputs {
if set.contains(input) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-explore/src/nu_common/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
create_table_for_record(cols, &records)
} else if cols.is_empty() && records.is_empty() {
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-json/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value>> {
pub fn as_array(&self) -> Option<&[Value]> {
match self {
Value::Array(array) => Some(array),
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-parser/src/parse_keywords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2887,7 +2887,7 @@ pub fn parse_overlay_hide(working_set: &mut StateWorkingSet, call: Box<Call>) ->

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;
Expand Down
27 changes: 14 additions & 13 deletions crates/nu-protocol/src/engine/engine_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand All @@ -363,7 +364,7 @@ impl EngineState {
pub fn active_overlay_names<'a, 'b>(
&'b self,
removed_overlays: &'a [Vec<u8>],
) -> impl DoubleEndedIterator<Item = &Vec<u8>> + 'a
) -> impl DoubleEndedIterator<Item = &[u8]> + 'a
where
'b: 'a,
{
Expand All @@ -389,7 +390,7 @@ impl EngineState {
.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)
.last()
.expect("internal error: no active overlays")
Expand All @@ -402,7 +403,7 @@ impl EngineState {
.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
.scope
.overlays
Expand Down Expand Up @@ -931,7 +932,7 @@ impl EngineState {
.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
}

Expand Down Expand Up @@ -1081,7 +1082,7 @@ impl StateDelta {
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
}
}
Expand Down Expand Up @@ -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<u8>> {
let mut names: HashSet<&Vec<u8>> = 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() {
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -1820,7 +1821,7 @@ impl<'a> StateWorkingSet<'a> {
.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![];

for scope_frame in self.delta.scope.iter().rev() {
Expand Down
10 changes: 7 additions & 3 deletions crates/nu-protocol/src/engine/overlay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -125,14 +129,14 @@ impl ScopeFrame {
.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)
.iter()
.map(|id| self.get_overlay_name(*id))
.collect()
}

pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &Vec<u8> {
pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] {
&self
.overlays
.get(overlay_id)
Expand Down

0 comments on commit e90b099

Please sign in to comment.