From a46048f36221f8ed19ddc019ce6e3d6b836ce314 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Wed, 1 Nov 2023 23:19:58 +0100 Subject: [PATCH] Use Record APIs in `nu-protocol`/`nu-engine` (#10917) # Description Consequences of #10841 This does not yet make the assumption that columns are always duplicated. Follow the existing logic here - Use saner record API in `nu-engine/src/eval.rs` - Use checked record construction in `nu-engine/src/scope.rs` - Use `values` iterator in `nu-engine/src/scope.rs` - Use `columns` iterator in `nu_engine::get_columns()` - Start using record API in `value/mod.rs` - Use `.insert` in `eval_const.rs` Record code - Record API for `eval_const.rs` table code # User-Facing Changes None # Tests + Formatting None --- crates/nu-engine/src/column.rs | 2 +- crates/nu-engine/src/eval.rs | 7 ++--- crates/nu-engine/src/scope.rs | 38 ++++++++++------------------ crates/nu-protocol/src/eval_const.rs | 16 +++--------- crates/nu-protocol/src/value/mod.rs | 4 +-- 5 files changed, 21 insertions(+), 46 deletions(-) diff --git a/crates/nu-engine/src/column.rs b/crates/nu-engine/src/column.rs index 23108d517..944a613f7 100644 --- a/crates/nu-engine/src/column.rs +++ b/crates/nu-engine/src/column.rs @@ -8,7 +8,7 @@ pub fn get_columns(input: &[Value]) -> Vec { return vec![]; }; - for col in &val.cols { + for col in val.columns() { if !columns.contains(col) { columns.push(col.to_string()); } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 2b6d3e374..e297a862b 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -552,7 +552,7 @@ pub fn eval_expression( for (col, val) in fields { // avoid duplicate cols. let col_name = eval_expression(engine_state, stack, col)?.as_string()?; - let pos = record.cols.iter().position(|c| c == &col_name); + let pos = record.index_of(&col_name); match pos { Some(index) => { return Err(ShellError::ColumnDefinedTwice { @@ -592,10 +592,7 @@ pub fn eval_expression( row.push(eval_expression(engine_state, stack, expr)?); } output_rows.push(Value::record( - Record { - cols: output_headers.clone(), - vals: row, - }, + Record::from_raw_cols_vals(output_headers.clone(), row), expr.span, )); } diff --git a/crates/nu-engine/src/scope.rs b/crates/nu-engine/src/scope.rs index 51a505e2a..3b0e0f4d6 100644 --- a/crates/nu-engine/src/scope.rs +++ b/crates/nu-engine/src/scope.rs @@ -198,9 +198,9 @@ impl<'e, 's> ScopeData<'e, 's> { // input sig_records.push(Value::record( - Record { - cols: sig_cols.clone(), - vals: vec![ + Record::from_raw_cols_vals( + sig_cols.clone(), + vec![ Value::nothing(span), Value::string("input", span), Value::string(input_type.to_shape().to_string(), span), @@ -210,7 +210,7 @@ impl<'e, 's> ScopeData<'e, 's> { Value::nothing(span), Value::nothing(span), ], - }, + ), span, )); @@ -231,10 +231,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record { - cols: sig_cols.clone(), - vals: sig_vals, - }, + Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), span, )); } @@ -260,10 +257,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record { - cols: sig_cols.clone(), - vals: sig_vals, - }, + Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), span, )); } @@ -285,10 +279,7 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record { - cols: sig_cols.clone(), - vals: sig_vals, - }, + Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), span, )); } @@ -335,19 +326,16 @@ impl<'e, 's> ScopeData<'e, 's> { ]; sig_records.push(Value::record( - Record { - cols: sig_cols.clone(), - vals: sig_vals, - }, + Record::from_raw_cols_vals(sig_cols.clone(), sig_vals), span, )); } // output sig_records.push(Value::record( - Record { - cols: sig_cols, - vals: vec![ + Record::from_raw_cols_vals( + sig_cols, + vec![ Value::nothing(span), Value::string("output", span), Value::string(output_type.to_shape().to_string(), span), @@ -357,7 +345,7 @@ impl<'e, 's> ScopeData<'e, 's> { Value::nothing(span), Value::nothing(span), ], - }, + ), span, )); @@ -587,7 +575,7 @@ fn sort_rows(decls: &mut [Value]) { (Value::Record { val: rec_a, .. }, Value::Record { val: rec_b, .. }) => { // Comparing the first value from the record // It is expected that the first value is the name of the entry (command, module, alias, etc.) - match (rec_a.vals.get(0), rec_b.vals.get(0)) { + match (rec_a.values().next(), rec_b.values().next()) { (Some(val_a), Some(val_b)) => match (val_a, val_b) { (Value::String { val: str_a, .. }, Value::String { val: str_b, .. }) => { str_a.cmp(str_b) diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 0fce15628..f062f2032 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -287,15 +287,7 @@ pub fn eval_constant( for (col, val) in fields { // avoid duplicate cols. let col_name = value_as_string(eval_constant(working_set, col)?, expr.span)?; - let pos = record.cols.iter().position(|c| c == &col_name); - match pos { - Some(index) => { - record.vals[index] = eval_constant(working_set, val)?; - } - None => { - record.push(col_name, eval_constant(working_set, val)?); - } - } + record.insert(col_name, eval_constant(working_set, val)?); } Ok(Value::record(record, expr.span)) @@ -323,11 +315,9 @@ pub fn eval_constant( for expr in val { row.push(eval_constant(working_set, expr)?); } + // length equality already ensured in parser output_rows.push(Value::record( - Record { - cols: output_headers.clone(), - vals: row, - }, + Record::from_raw_cols_vals(output_headers.clone(), row), expr.span, )); } diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index f495b7cf8..72cf6d20b 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -3113,7 +3113,7 @@ impl Value { } (lhs, Value::List { vals: rhs, .. }) => Ok(Value::bool(rhs.contains(lhs), span)), (Value::String { val: lhs, .. }, Value::Record { val: rhs, .. }) => { - Ok(Value::bool(rhs.cols.contains(lhs), span)) + Ok(Value::bool(rhs.contains(lhs), span)) } (Value::String { .. } | Value::Int { .. }, Value::CellPath { val: rhs, .. }) => { let val = rhs.members.iter().any(|member| match (self, member) { @@ -3161,7 +3161,7 @@ impl Value { } (lhs, Value::List { vals: rhs, .. }) => Ok(Value::bool(!rhs.contains(lhs), span)), (Value::String { val: lhs, .. }, Value::Record { val: rhs, .. }) => { - Ok(Value::bool(!rhs.cols.contains(lhs), span)) + Ok(Value::bool(!rhs.contains(lhs), span)) } (Value::String { .. } | Value::Int { .. }, Value::CellPath { val: rhs, .. }) => { let val = rhs.members.iter().any(|member| match (self, member) {