From 9dc4a60ef20b54a7246888e0212cd3053f25c0ac Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Wed, 10 Apr 2019 22:34:01 -0400
Subject: [PATCH] Preallocate space and reduce returns
---
src/lib/shell/flow.rs | 142 +++++++-------------
src/lib/shell/flow_control.rs | 246 +++++++++++++++++-----------------
2 files changed, 169 insertions(+), 219 deletions(-)
diff --git a/src/lib/shell/flow.rs b/src/lib/shell/flow.rs
index 968fe600..4fff77e3 100644
--- a/src/lib/shell/flow.rs
+++ b/src/lib/shell/flow.rs
@@ -18,19 +18,8 @@ use crate::{
};
use itertools::Itertools;
use small;
-use std::io::{stdout, Write};
-
-macro_rules! handle_signal {
- ($signal:expr) => {
- match $signal {
- Condition::Break => break,
- Condition::SigInt => return Condition::SigInt,
- _ => (),
- }
- };
-}
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
pub(crate) enum Condition {
Continue,
Break,
@@ -120,7 +109,11 @@ impl FlowLogic for Shell {
}
}
- handle_signal!(self.execute_statements(statements));
+ match self.execute_statements(statements) {
+ Condition::Break => break,
+ Condition::SigInt => return Condition::SigInt,
+ Condition::Continue | Condition::NoOp => (),
+ }
};
}
@@ -149,13 +142,13 @@ impl FlowLogic for Shell {
fn execute_while(&mut self, expression: &[Statement], statements: &[Statement]) -> Condition {
loop {
- self.execute_statements(&expression);
+ self.execute_statements(expression);
if self.previous_status != 0 {
return Condition::NoOp;
}
// Cloning is needed so the statement can be re-iterated again if needed.
- match self.execute_statements(&statements) {
+ match self.execute_statements(statements) {
Condition::Break => return Condition::NoOp,
Condition::SigInt => return Condition::SigInt,
_ => (),
@@ -179,21 +172,20 @@ impl FlowLogic for Shell {
self.variables.set("?", self.previous_status.to_string());
}
Statement::While { expression, statements } => {
- if let Condition::SigInt = self.execute_while(&expression, &statements) {
+ if self.execute_while(&expression, &statements) == Condition::SigInt {
return Condition::SigInt;
}
}
Statement::For { variables, values, statements } => {
- if let Condition::SigInt = self.execute_for(&variables, &values, &statements) {
+ if self.execute_for(&variables, &values, &statements) == Condition::SigInt {
return Condition::SigInt;
}
}
Statement::If { expression, success, else_if, failure, .. } => {
- match self.execute_if(&expression, &success, &else_if, &failure) {
- Condition::Break => return Condition::Break,
- Condition::Continue => return Condition::Continue,
- Condition::NoOp => (),
- Condition::SigInt => return Condition::SigInt,
+ let condition = self.execute_if(&expression, &success, &else_if, &failure);
+
+ if condition != Condition::NoOp {
+ return condition;
}
}
Statement::Function { name, args, statements, description } => {
@@ -229,7 +221,7 @@ impl FlowLogic for Shell {
}
},
Statement::Time(box_statement) => {
- let time = ::std::time::Instant::now();
+ let time = std::time::Instant::now();
let condition = self.execute_statement(box_statement);
@@ -237,24 +229,13 @@ impl FlowLogic for Shell {
let seconds = duration.as_secs();
let nanoseconds = duration.subsec_nanos();
- let stdout = stdout();
- let mut stdout = stdout.lock();
- let _ = if seconds > 60 {
- writeln!(
- stdout,
- "real {}m{:02}.{:09}s",
- seconds / 60,
- seconds % 60,
- nanoseconds
- )
+ if seconds > 60 {
+ println!("real {}m{:02}.{:09}s", seconds / 60, seconds % 60, nanoseconds);
} else {
- writeln!(stdout, "real {}.{:09}s", seconds, nanoseconds)
- };
- match condition {
- Condition::Break => return Condition::Break,
- Condition::Continue => return Condition::Continue,
- Condition::NoOp => (),
- Condition::SigInt => return Condition::SigInt,
+ println!("real {}.{:09}s", seconds, nanoseconds);
+ }
+ if condition != Condition::NoOp {
+ return condition;
}
}
Statement::And(box_statement) => {
@@ -263,11 +244,8 @@ impl FlowLogic for Shell {
_ => Condition::NoOp,
};
- match condition {
- Condition::Break => return Condition::Break,
- Condition::Continue => return Condition::Continue,
- Condition::NoOp => (),
- Condition::SigInt => return Condition::SigInt,
+ if condition != Condition::NoOp {
+ return condition;
}
}
Statement::Or(box_statement) => {
@@ -276,11 +254,8 @@ impl FlowLogic for Shell {
_ => Condition::NoOp,
};
- match condition {
- Condition::Break => return Condition::Break,
- Condition::Continue => return Condition::Continue,
- Condition::NoOp => (),
- Condition::SigInt => return Condition::SigInt,
+ if condition != Condition::NoOp {
+ return condition;
}
}
Statement::Not(box_statement) => {
@@ -297,11 +272,10 @@ impl FlowLogic for Shell {
Statement::Break => return Condition::Break,
Statement::Continue => return Condition::Continue,
Statement::Match { expression, cases } => {
- match self.execute_match(expression, &cases) {
- Condition::Break => return Condition::Break,
- Condition::Continue => return Condition::Continue,
- Condition::NoOp => (),
- Condition::SigInt => return Condition::SigInt,
+ let condition = self.execute_match(expression, &cases);
+
+ if condition != Condition::NoOp {
+ return condition;
}
}
_ => {}
@@ -322,20 +296,15 @@ impl FlowLogic for Shell {
fn execute_statements(&mut self, statements: &[Statement]) -> Condition {
self.variables.new_scope(false);
- let mut condition = None;
- for statement in statements {
- match self.execute_statement(statement) {
- Condition::NoOp => {}
- cond => {
- condition = Some(cond);
- break;
- }
- }
- }
+ let condition = statements
+ .iter()
+ .map(|statement| self.execute_statement(statement))
+ .find(|&condition| condition != Condition::NoOp)
+ .unwrap_or(Condition::NoOp);
self.variables.pop_scope();
- condition.unwrap_or(Condition::NoOp)
+ condition
}
fn execute_match<T: AsRef<str>>(&mut self, expression: T, cases: &[Case]) -> Condition {
@@ -424,13 +393,12 @@ fn expand_pipeline(
shell: &Shell,
pipeline: &Pipeline,
) -> Result<(Pipeline, Vec<Statement>), String> {
- let mut item_iter = pipeline.items.iter();
- let mut items: Vec<PipeItem> = Vec::new();
+ let mut item_iter = pipeline.items.iter().cloned();
+ let mut items: Vec<PipeItem> = Vec::with_capacity(item_iter.size_hint().0);
let mut statements = Vec::new();
while let Some(item) = item_iter.next() {
- let possible_alias = shell.variables.get::<types::Alias>(item.job.command.as_ref());
- if let Some(alias) = possible_alias {
+ if let Some(alias) = shell.variables.get::<types::Alias>(item.job.command.as_ref()) {
statements = StatementSplitter::new(alias.0.as_str()).map(parse_and_validate).collect();
// First item in the alias should be a pipeline item, otherwise it cannot
@@ -442,14 +410,10 @@ fn expand_pipeline(
first.inputs = item.inputs.clone();
// Add alias arguments to expanded args if there's any.
- if item.job.args.len() > 1 {
- for arg in &item.job.args[1..] {
- first.job.args.push(arg.clone());
- }
- }
+ first.job.args.extend(item.job.args.iter().skip(1).cloned());
}
if len == 1 {
- if let Some(last) = pline.items.last_mut() {
+ if let Some(mut last) = pline.items.last_mut() {
last.outputs = item.outputs.clone();
last.job.kind = item.job.kind;
}
@@ -460,13 +424,12 @@ fn expand_pipeline(
// Handle pipeline being broken half by i.e.: '&&' or '||'
if !statements.is_empty() {
- let err = match statements.last_mut().unwrap() {
+ match statements.last_mut().unwrap() {
Statement::And(ref mut boxed_stm)
| Statement::Or(ref mut boxed_stm)
| Statement::Not(ref mut boxed_stm)
| Statement::Time(ref mut boxed_stm) => {
- let stm = &mut **boxed_stm;
- if let Statement::Pipeline(ref mut pline) = stm {
+ if let Statement::Pipeline(ref mut pline) = &mut **boxed_stm {
// Set output of alias to be the output of last pipeline.
if let Some(last) = pline.items.last_mut() {
last.outputs = item.outputs.clone();
@@ -474,24 +437,17 @@ fn expand_pipeline(
}
// Append rest of the pipeline to the last pipeline in the
// alias.
- for item in item_iter {
- pline.items.push(item.clone());
- }
- // No error
- false
+ pline.items.extend(item_iter);
} else {
// Error in expansion
- true
+ return Err(format!(
+ "unable to pipe outputs of alias: '{} = {}'",
+ item.job.command.as_str(),
+ alias.0.as_str()
+ ));
}
}
- _ => false,
- };
- if err {
- return Err(format!(
- "unable to pipe outputs of alias: '{} = {}'",
- item.job.command.as_str(),
- alias.0.as_str()
- ));
+ _ => (),
}
break;
}
diff --git a/src/lib/shell/flow_control.rs b/src/lib/shell/flow_control.rs
index 81e54377..1fd6372d 100644
--- a/src/lib/shell/flow_control.rs
+++ b/src/lib/shell/flow_control.rs
@@ -179,177 +179,173 @@ pub(crate) fn insert_statement(
| Statement::While { .. }
| Statement::Match { .. }
| Statement::If { .. }
- | Statement::Function { .. } => flow_control.block.push(statement),
+ | Statement::Function { .. } => {
+ flow_control.block.push(statement);
+ Ok(None)
+ }
// Case is special as it should pop back previous Case
Statement::Case(_) => {
- let mut top_is_case = false;
match flow_control.block.last() {
- Some(Statement::Case(_)) => top_is_case = true,
+ Some(Statement::Case(_)) => {
+ let case = flow_control.block.pop().unwrap();
+ let _ = insert_into_block(&mut flow_control.block, case);
+ }
Some(Statement::Match { .. }) => (),
_ => return Err("ion: error: Case { .. } found outside of Match { .. } block"),
}
- if top_is_case {
- let case = flow_control.block.pop().unwrap();
- let _ = insert_into_block(&mut flow_control.block, case);
- }
flow_control.block.push(statement);
+ Ok(None)
}
Statement::End => {
match flow_control.block.len() {
- 0 => return Err("ion: error: keyword End found but no block to close"),
+ 0 => Err("ion: error: keyword End found but no block to close"),
// Ready to return the complete block
- 1 => return Ok(flow_control.block.pop()),
+ 1 => Ok(flow_control.block.pop()),
// Merge back the top block into the previous one
_ => {
let block = flow_control.block.pop().unwrap();
- match block {
- Statement::Case(_) => {
- // Merge last Case back and pop off Match too
- insert_into_block(&mut flow_control.block, block)?;
- let match_stm = flow_control.block.pop().unwrap();
- if !flow_control.block.is_empty() {
- insert_into_block(&mut flow_control.block, match_stm)?;
- } else {
- return Ok(Some(match_stm));
- }
+ let mut case = false;
+ if let Statement::Case(_) = block {
+ case = true;
+ }
+ insert_into_block(&mut flow_control.block, block)?;
+ if case {
+ // Merge last Case back and pop off Match too
+ let match_stm = flow_control.block.pop().unwrap();
+ if !flow_control.block.is_empty() {
+ insert_into_block(&mut flow_control.block, match_stm)?;
+
+ Ok(None)
+ } else {
+ Ok(Some(match_stm))
}
- _ => insert_into_block(&mut flow_control.block, block)?,
+ } else {
+ Ok(None)
}
}
}
}
Statement::And(_) | Statement::Or(_) if !flow_control.block.is_empty() => {
let mut pushed = true;
- if let Some(top) = flow_control.block.last_mut() {
- match top {
- Statement::If {
- ref mut expression,
- ref mode,
- ref success,
- ref mut else_if,
- ..
- } => match *mode {
- 0 if success.is_empty() => {
- // Insert into If expression if there's no previous statement.
- expression.push(statement.clone());
- }
- 1 => {
- // Try to insert into last ElseIf expression if there's no previous
- // statement.
- if let Some(eif) = else_if.last_mut() {
- if eif.success.is_empty() {
- eif.expression.push(statement.clone());
- } else {
- pushed = false;
- }
+ match flow_control.block.last_mut().unwrap() {
+ Statement::If {
+ ref mut expression,
+ ref mode,
+ ref success,
+ ref mut else_if,
+ ..
+ } => match *mode {
+ 0 if success.is_empty() => {
+ // Insert into If expression if there's no previous statement.
+ expression.push(statement.clone());
+ }
+ 1 => {
+ // Try to insert into last ElseIf expression if there's no previous
+ // statement.
+ if let Some(eif) = else_if.last_mut() {
+ if eif.success.is_empty() {
+ eif.expression.push(statement.clone());
} else {
- // should not be reached...
- unreachable!("Missmatch in 'If' mode!")
+ pushed = false;
}
- }
- _ => pushed = false,
- },
- Statement::While { ref mut expression, ref statements } => {
- if statements.is_empty() {
- expression.push(statement.clone());
} else {
- pushed = false;
+ // should not be reached...
+ unreachable!("Missmatch in 'If' mode!")
}
}
_ => pushed = false,
+ },
+ Statement::While { ref mut expression, ref statements } => {
+ if statements.is_empty() {
+ expression.push(statement.clone());
+ } else {
+ pushed = false;
+ }
}
- } else {
- unreachable!()
+ _ => pushed = false,
}
if !pushed {
insert_into_block(&mut flow_control.block, statement)?;
}
+
+ Ok(None)
}
Statement::Time(inner) => {
if inner.is_block() {
flow_control.block.push(Statement::Time(inner));
+ Ok(None)
} else {
- return Ok(Some(Statement::Time(inner)));
+ Ok(Some(Statement::Time(inner)))
}
}
_ => {
if !flow_control.block.is_empty() {
insert_into_block(&mut flow_control.block, statement)?;
+ Ok(None)
} else {
// Filter out toplevel statements that should produce an error
// otherwise return the statement for immediat execution
match statement {
Statement::ElseIf(_) => {
- return Err("ion: error: found ElseIf { .. } without If { .. } block");
- }
- Statement::Else => return Err("ion: error: found Else without If { .. } block"),
- Statement::Break => return Err("ion: error: found Break without loop body"),
- Statement::Continue => {
- return Err("ion: error: found Continue without loop body");
+ Err("ion: error: found ElseIf { .. } without If { .. } block")
}
+ Statement::Else => Err("ion: error: found Else without If { .. } block"),
+ Statement::Break => Err("ion: error: found Break without loop body"),
+ Statement::Continue => Err("ion: error: found Continue without loop body"),
// Toplevel statement, return to execute immediately
- _ => return Ok(Some(statement)),
+ _ => Ok(Some(statement)),
}
}
}
}
- Ok(None)
}
fn insert_into_block(block: &mut Vec<Statement>, statement: Statement) -> Result<(), &'static str> {
- if let Some(top_block) = block.last_mut() {
- let block = match top_block {
- Statement::Time(inner) => inner,
- _ => top_block,
- };
-
- match block {
- Statement::Function { ref mut statements, .. } => statements.push(statement),
- Statement::For { ref mut statements, .. } => statements.push(statement),
- Statement::While { ref mut statements, .. } => statements.push(statement),
- Statement::Match { ref mut cases, .. } => match statement {
- Statement::Case(case) => cases.push(case),
- _ => {
- return Err(
- "ion: error: statement found outside of Case { .. } block in Match { .. }"
- );
- }
- },
- Statement::Case(ref mut case) => case.statements.push(statement),
- Statement::If {
- ref mut success,
- ref mut else_if,
- ref mut failure,
- ref mut mode,
- ..
- } => match statement {
- Statement::ElseIf(eif) => {
- if *mode > 1 {
- return Err("ion: error: ElseIf { .. } found after Else");
- } else {
- *mode = 1;
- else_if.push(eif);
- }
+ let block = match block.last_mut().expect("Should not insert statement if stack is empty!") {
+ Statement::Time(inner) => inner,
+ top_block @ _ => top_block,
+ };
+
+ match block {
+ Statement::Function { ref mut statements, .. } => statements.push(statement),
+ Statement::For { ref mut statements, .. } => statements.push(statement),
+ Statement::While { ref mut statements, .. } => statements.push(statement),
+ Statement::Match { ref mut cases, .. } => match statement {
+ Statement::Case(case) => cases.push(case),
+ _ => {
+ return Err(
+ "ion: error: statement found outside of Case { .. } block in Match { .. }"
+ );
+ }
+ },
+ Statement::Case(ref mut case) => case.statements.push(statement),
+ Statement::If {
+ ref mut success, ref mut else_if, ref mut failure, ref mut mode, ..
+ } => match statement {
+ Statement::ElseIf(eif) => {
+ if *mode > 1 {
+ return Err("ion: error: ElseIf { .. } found after Else");
+ } else {
+ *mode = 1;
+ else_if.push(eif);
}
- Statement::Else => {
- if *mode == 2 {
- return Err("ion: error: Else block already exists");
- } else {
- *mode = 2;
- }
+ }
+ Statement::Else => {
+ if *mode == 2 {
+ return Err("ion: error: Else block already exists");
+ } else {
+ *mode = 2;
}
- _ => match *mode {
- 0 => success.push(statement),
- 1 => else_if.last_mut().unwrap().success.push(statement),
- 2 => failure.push(statement),
- _ => unreachable!(),
- },
+ }
+ _ => match *mode {
+ 0 => success.push(statement),
+ 1 => else_if.last_mut().unwrap().success.push(statement),
+ 2 => failure.push(statement),
+ _ => unreachable!(),
},
- _ => unreachable!("Not block-like statement pushed to stack!"),
- }
- } else {
- unreachable!("Should not insert statement if stack is empty!")
+ },
+ _ => unreachable!("Not block-like statement pushed to stack!"),
}
Ok(())
}
@@ -390,27 +386,25 @@ impl Function {
return Err(FunctionError::InvalidArgumentCount);
}
- let name = self.name.clone();
-
- let mut values: SmallVec<[_; 8]> = SmallVec::new();
-
- for (type_, value) in self.args.iter().zip(args.iter().skip(1)) {
- let value = match value_check(shell, value.as_ref(), &type_.kind) {
- Ok(value) => value,
- Err(_) => {
- return Err(FunctionError::InvalidArgumentType(
+ let values = self
+ .args
+ .iter()
+ .zip(args.iter().skip(1))
+ .map(|(type_, value)| {
+ if let Ok(value) = value_check(shell, value.as_ref(), &type_.kind) {
+ Ok((type_.clone(), value))
+ } else {
+ Err(FunctionError::InvalidArgumentType(
type_.kind.clone(),
value.as_ref().into(),
- ));
+ ))
}
- };
-
- values.push((type_.clone(), value));
- }
+ })
+ .collect::<Result<SmallVec<[_; 8]>, _>>()?;
let index = shell
.variables
- .index_scope_for_var(&name)
+ .index_scope_for_var(&self.name)
.expect("execute called with invalid function");
// Pop off all scopes since function temporarily
--
GitLab