From 09f1dff0440bddc41034d0567080a35d2904e404 Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Mon, 25 Mar 2019 20:31:16 +0000
Subject: [PATCH] Reorder if statement to extract common patterns and allow
collecting
Reduce the number of possible branches.
Use collect instead of Vec::new for collecting values faster and with
less annotations
---
ci/run_benchmark.sh | 5 +-
src/lib/parser/quotes.rs | 202 +++++++++++++++---------------
src/lib/shell/binary/mod.rs | 32 ++---
src/lib/shell/binary/readln.rs | 4 +-
src/lib/shell/binary/terminate.rs | 6 +-
src/lib/shell/completer.rs | 84 ++++++-------
src/lib/shell/mod.rs | 13 +-
src/lib/shell/variables/mod.rs | 88 +++++--------
8 files changed, 197 insertions(+), 237 deletions(-)
diff --git a/ci/run_benchmark.sh b/ci/run_benchmark.sh
index e6f02a57..bc7b8080 100755
--- a/ci/run_benchmark.sh
+++ b/ci/run_benchmark.sh
@@ -7,7 +7,7 @@ cargo bench
cargo build --release
PREV_SIZE=$(ls -al target/release/ion | cut -d' ' -f5)
-git stash push
+git stash
git checkout -
cargo bench
cargo build --release
@@ -31,7 +31,8 @@ for suite in ./target/criterion/*; do
for test in $suite/*/*/change/estimates.json; do
estimate=$(cat "$test" | jq -r "$JQ_FILTER" -c)
case "$estimate" in
- -*)
+ -*);;
+ *)
inner="<failure message=\"Performance Regressed\" type=\"WARNING\">\
Performance regressed by $estimate in $test\
</failure>"
diff --git a/src/lib/parser/quotes.rs b/src/lib/parser/quotes.rs
index 2b080d57..c4a782a6 100644
--- a/src/lib/parser/quotes.rs
+++ b/src/lib/parser/quotes.rs
@@ -48,6 +48,8 @@ pub struct Terminator<I: Iterator<Item = u8>> {
terminated: bool,
and_or: bool,
whitespace: bool,
+ lt_count: u8,
+ empty: bool,
}
impl<'a> From<&'a str> for Terminator<std::str::Bytes<'a>> {
@@ -99,125 +101,127 @@ impl<I: Iterator<Item = u8>> Iterator for Terminator<I> {
return None;
}
- let next = self.inner.next();
- let out = self.handle_char(next);
+ let prev_whitespace = self.whitespace;
+ self.whitespace = false;
- if out.is_none()
- && self.eof.is_none()
- && self.array == 0
- && self.quotes == Quotes::None
- && !self.and_or
- {
- self.terminated = true;
- None
+ let mut next = if prev_whitespace && self.array == 0 && !self.and_or && !self.empty {
+ self.inner.find(|&c| c == b'\n' || !c.is_ascii_whitespace())
+ } else if prev_whitespace {
+ self.inner.find(|&c| !c.is_ascii_whitespace())
} else {
- out
+ self.inner.next()
+ };
+
+ if self.skip_next {
+ self.skip_next = false;
+ } else if let Some(matcher) = self.eof.as_mut() {
+ if let Some(character) = next {
+ if matcher.next(character) {
+ self.eof = None;
+ }
+ }
+ } else if self.quotes != Quotes::None && next != Some(b'\\') {
+ match (next, &self.quotes) {
+ (Some(b'\''), Quotes::Single) | (Some(b'"'), Quotes::Double) => {
+ self.quotes = Quotes::None;
+ }
+ _ => (),
+ }
+ } else if let Some(character) = next {
+ next = self.handle_char(character, prev_whitespace);
+ self.empty &= character.is_ascii_whitespace();
+ } else if self.array == 0 && !self.and_or && !self.empty {
+ self.terminated = true;
}
+
+ next
}
}
impl<I: Iterator<Item = u8>> Terminator<I> {
/// Consumes lines until a statement is formed or the iterator runs dry, and returns the
/// underlying `String`.
- pub fn terminate(&mut self) -> Result<String, ()> {
+ pub fn terminate(&mut self) -> Option<Result<String, ()>> {
let stmt = self.collect::<Vec<_>>();
let stmt = unsafe { String::from_utf8_unchecked(stmt) };
if self.terminated {
- Ok(stmt)
+ Some(Ok(stmt))
+ } else if self.empty {
+ None
} else {
- Err(())
+ Some(Err(()))
}
}
- fn handle_char(&mut self, character: Option<u8>) -> Option<u8> {
- character.and_then(|character| {
- let prev_whitespace = self.whitespace;
- self.whitespace = false;
-
- if let Some(matcher) = self.eof.as_mut() {
- if matcher.next(character) {
- self.eof = None;
+ fn handle_char(&mut self, character: u8, prev_whitespace: bool) -> Option<u8> {
+ match character {
+ b'\'' => {
+ self.quotes = Quotes::Single;
+ Some(b'\'')
+ }
+ b'"' => {
+ self.quotes = Quotes::Double;
+ Some(b'"')
+ }
+ b'<' => {
+ if let Some(&b'<') = self.inner.peek() {
+ self.lt_count += 1;
+ } else if self.lt_count == 1 {
+ self.eof = Some(EofMatcher::new());
+ self.lt_count = 0;
+ } else {
+ self.lt_count = 0;
}
- Some(character)
- } else if self.skip_next {
- self.skip_next = false;
- Some(character)
- } else if self.quotes != Quotes::None && character != b'\\' {
- match (character, &self.quotes) {
- (b'\'', Quotes::Single) | (b'"', Quotes::Double) => {
- self.quotes = Quotes::None;
- }
- _ => (),
+ Some(b'<')
+ }
+ b'[' => {
+ self.array += 1;
+ Some(b'[')
+ }
+ b']' => {
+ if self.array > 0 {
+ self.array -= 1;
}
- Some(character)
- } else {
- match character {
- b'\'' => {
- self.quotes = Quotes::Single;
- Some(b'\'')
- }
- b'"' => {
- self.quotes = Quotes::Double;
- Some(b'"')
- }
- b'<' if self.inner.prev() == Some(&b'<') => {
- if let Some(&b'<') = self.inner.peek() {
- self.skip_next = true; // avoid falling in the else at the next pass
- } else {
- self.eof = Some(EofMatcher::new());
- }
- Some(b'<')
- }
- b'[' => {
- self.array += 1;
- Some(b'[')
- }
- b']' => {
- if self.array > 0 {
- self.array -= 1;
- }
- Some(b']')
- }
- b'#' if self.inner.prev().filter(|&c| ![b' ', b'\n'].contains(c)).is_none() => {
- self.whitespace = prev_whitespace;
- let next = self.inner.find(|&c| c == b'\n');
- self.handle_char(next)
- }
- b'\\' => {
- if self.inner.peek() == Some(&b'\n') {
- let next = self.inner.find(|&c| !(c as char).is_whitespace());
- self.handle_char(next)
- } else {
- self.skip_next = true;
- Some(character)
- }
- }
- b'&' | b'|' if self.inner.prev() == Some(&character) => {
- self.and_or = true;
- Some(character)
- }
- b'\n' if self.array == 0 && !self.and_or => {
- self.terminated = true;
- None
- }
- _ if (character as char).is_whitespace() => {
- if prev_whitespace {
- let next =
- self.inner.find(|&c| c == b'\n' || !(c as char).is_whitespace());
- self.handle_char(next)
- } else {
- self.whitespace = true;
- Some(b' ')
- }
- }
- _ => {
- self.and_or = false;
- Some(character)
- }
+ Some(b']')
+ }
+ b'#' if prev_whitespace => {
+ self.inner.find(|&c| c == b'\n');
+ if self.array == 0 && !self.and_or && !self.empty {
+ self.terminated = true;
+ None
+ } else {
+ self.whitespace = true;
+ Some(b' ')
+ }
+ }
+ b'\\' => {
+ if self.inner.peek() == Some(&b'\n') {
+ self.whitespace = true;
+ self.inner.next();
+ self.next()
+ } else {
+ self.skip_next = true;
+ Some(character)
}
}
- })
+ b'&' | b'|' if self.inner.prev() == Some(&character) => {
+ self.and_or = true;
+ Some(character)
+ }
+ b'\n' if self.array == 0 && !self.and_or && !self.empty => {
+ self.terminated = true;
+ None
+ }
+ _ if character.is_ascii_whitespace() => {
+ self.whitespace = true;
+ Some(b' ')
+ }
+ _ => {
+ self.and_or = false;
+ Some(character)
+ }
+ }
}
pub fn new(inner: I) -> Terminator<I> {
@@ -230,6 +234,8 @@ impl<I: Iterator<Item = u8>> Terminator<I> {
terminated: false,
and_or: false,
whitespace: true,
+ lt_count: 0,
+ empty: true,
}
}
}
diff --git a/src/lib/shell/binary/mod.rs b/src/lib/shell/binary/mod.rs
index 7faba21a..0984c41a 100644
--- a/src/lib/shell/binary/mod.rs
+++ b/src/lib/shell/binary/mod.rs
@@ -51,21 +51,12 @@ pub trait Binary {
impl Binary for Shell {
fn save_command(&mut self, cmd: &str) {
- if cmd.starts_with('~') {
- if !cmd.ends_with('/')
- && self
- .variables
- .tilde_expansion(cmd, &self.directory_stack)
- .map_or(false, |path| Path::new(&path).is_dir())
- {
- self.save_command_in_history(&[cmd, "/"].concat());
- } else {
- self.save_command_in_history(cmd);
- }
- return;
- }
-
- if Path::new(cmd).is_dir() & !cmd.ends_with('/') {
+ if !cmd.ends_with('/')
+ && self
+ .variables
+ .tilde_expansion(cmd, &self.directory_stack)
+ .map_or(false, |path| Path::new(&path).is_dir())
+ {
self.save_command_in_history(&[cmd, "/"].concat());
} else {
self.save_command_in_history(cmd);
@@ -91,17 +82,20 @@ impl Binary for Shell {
self.evaluate_init_file();
loop {
- let mut lines = itertools::repeat_call(|| self.readln())
+ let mut lines = std::iter::repeat_with(|| self.readln())
.filter_map(|cmd| cmd)
.flat_map(|s| s.into_bytes().into_iter().chain(Some(b'\n')));
- match Terminator::new(&mut lines).terminate().ok() {
- Some(command) => {
+ match Terminator::new(&mut lines).terminate() {
+ Some(Ok(command)) => {
self.flags &= !UNTERMINATED;
let cmd: &str = &designators::expand_designators(&self, command.trim_end());
self.on_command(&cmd);
self.save_command(&cmd);
}
- None => self.reset_flow(),
+ Some(Err(_)) => self.reset_flow(),
+ None => {
+ self.flags &= !UNTERMINATED;
+ }
}
}
}
diff --git a/src/lib/shell/binary/readln.rs b/src/lib/shell/binary/readln.rs
index e1739627..054763d1 100644
--- a/src/lib/shell/binary/readln.rs
+++ b/src/lib/shell/binary/readln.rs
@@ -111,7 +111,9 @@ pub(crate) fn readln(shell: &mut Shell) -> Option<String> {
match line {
Ok(line) => {
- shell.flags |= flags::UNTERMINATED;
+ if !line.is_empty() {
+ shell.flags |= flags::UNTERMINATED;
+ }
Some(line)
}
// Handles Ctrl + C
diff --git a/src/lib/shell/binary/terminate.rs b/src/lib/shell/binary/terminate.rs
index 45fe5f46..b4017b9f 100644
--- a/src/lib/shell/binary/terminate.rs
+++ b/src/lib/shell/binary/terminate.rs
@@ -2,11 +2,11 @@ use crate::{
parser::Terminator,
shell::{status::*, FlowLogic, Shell},
};
+use itertools::Itertools;
pub(crate) fn terminate_script_quotes<I: Iterator<Item = u8>>(shell: &mut Shell, lines: I) -> i32 {
- let mut lines = lines.peekable();
- while lines.peek().is_some() {
- match Terminator::new(&mut lines).terminate() {
+ for cmd in lines.batching(|lines| Terminator::new(lines).terminate()) {
+ match cmd {
Ok(stmt) => shell.on_command(&stmt),
Err(_) => {
eprintln!("ion: unterminated quote in script");
diff --git a/src/lib/shell/completer.rs b/src/lib/shell/completer.rs
index ea4cee21..28dd1f79 100644
--- a/src/lib/shell/completer.rs
+++ b/src/lib/shell/completer.rs
@@ -41,60 +41,48 @@ impl Completer for IonFileCompleter {
/// and then escape the resulting filenames, as well as remove the expanded form of the `~`
/// character and re-add the `~` character in it's place.
fn completions(&self, start: &str) -> Vec<String> {
- // Only if the first character is a tilde character will we perform expansions
- if start.starts_with('~') {
- // Dereferencing the raw pointers here should be entirely safe, theoretically,
- // because no changes will occur to either of the underlying references in the
- // duration between creation of the completers and execution of their
- // completions.
- if let Some(expanded) = unsafe { (*self.vars).tilde_expansion(start, &*self.dir_stack) }
- {
- // Now we obtain completions for the `expanded` form of the `start` value.
- let mut iterator = filename_completion(&expanded, |x| self.inner.completions(x));
-
+ // Dereferencing the raw pointers here should be entirely safe, theoretically,
+ // because no changes will occur to either of the underlying references in the
+ // duration between creation of the completers and execution of their
+ // completions.
+ if let Some(expanded) = unsafe { (*self.vars).tilde_expansion(start, &*self.dir_stack) } {
+ // Now we obtain completions for the `expanded` form of the `start` value.
+ let iterator = filename_completion(&expanded, |x| self.inner.completions(x));
+
+ // We can do that by obtaining the index position where the tilde character
+ // ends. We don't search with `~` because we also want to
+ // handle other tilde variants.
+ let t_index = start.bytes().find(|&c| c == b'/').unwrap_or(1);
+ // `tilde` is the tilde pattern, and `search` is the pattern that follows.
+ let (tilde, search) = start.split_at(t_index as usize);
+
+ if search.len() < 2 {
+ // If the length of the search pattern is less than 2, the search pattern is
+ // empty, and thus the completions actually contain files and directories in
+ // the home directory.
+
+ // The tilde pattern will actually be our `start` command in itself,
+ // and the completed form will be all of the characters beyond the length of
+ // the expanded form of the tilde pattern.
+ iterator.map(|completion| [start, &completion[expanded.len()..]].concat()).collect()
+ // To save processing time, we should get obtain the index position where our
+ // search pattern begins, and re-use that index to slice the completions so
+ // that we may re-add the tilde character with the completion that follows.
+ } else if let Some(e_index) = expanded.rfind(search) {
// And then we will need to take those completions and remove the expanded form
// of the tilde pattern and replace it with that pattern yet again.
- let mut completions = Vec::new();
-
- // We can do that by obtaining the index position where the tilde character
- // ends. We don't search with `~` because we also want to
- // handle other tilde variants.
- let t_index = start.find('/').unwrap_or(1);
- // `tilde` is the tilde pattern, and `search` is the pattern that follows.
- let (tilde, search) = start.split_at(t_index as usize);
-
- if search.len() < 2 {
- // If the length of the search pattern is less than 2, the search pattern is
- // empty, and thus the completions actually contain files and directories in
- // the home directory.
-
- // The tilde pattern will actually be our `start` command in itself,
- // and the completed form will be all of the characters beyond the length of
- // the expanded form of the tilde pattern.
- for completion in iterator {
- completions.push([start, &completion[expanded.len()..]].concat());
- }
- // To save processing time, we should get obtain the index position where our
- // search pattern begins, and re-use that index to slice the completions so
- // that we may re-add the tilde character with the completion that follows.
- } else if let Some(completion) = iterator.next() {
- if let Some(e_index) = expanded.rfind(search) {
- completions.push(escape(&[tilde, &completion[e_index..]].concat()));
- for completion in iterator {
- let expanded = &completion[e_index..];
- completions.push(escape(&[tilde, expanded].concat()));
- }
- }
- }
-
- return completions;
+ iterator
+ .map(|completion| escape(&[tilde, &completion[e_index..]].concat()))
+ .collect()
+ } else {
+ Vec::new()
}
} else if start.starts_with("./") && unescape(start).split('/').count() == 2 {
// Special case for ./scripts, the globbing code removes the ./
- return self.inner.completions(&start);
+ self.inner.completions(&start)
+ } else {
+ filename_completion(&start, |x| self.inner.completions(x)).collect()
}
-
- filename_completion(&start, |x| self.inner.completions(x)).collect()
}
}
diff --git a/src/lib/shell/mod.rs b/src/lib/shell/mod.rs
index 59e1f0bf..d5aa809a 100644
--- a/src/lib/shell/mod.rs
+++ b/src/lib/shell/mod.rs
@@ -60,6 +60,7 @@ use crate::{
sys,
types::{self, Array},
};
+use itertools::Itertools;
use liner::Context;
use std::{
fs,
@@ -239,13 +240,13 @@ impl Shell {
where
T: 'a + AsRef<str> + std::clone::Clone + std::convert::From<&'a str>,
{
- let mut terminator: Terminator<_> = command.as_ref().into();
- if let Ok(stmt) = terminator.terminate() {
- self.on_command(&stmt);
- Ok(self.previous_status)
- } else {
- Err(IonError::Unterminated)
+ for cmd in command.as_ref().bytes().batching(|bytes| Terminator::new(bytes).terminate()) {
+ match cmd {
+ Ok(stmt) => self.on_command(&stmt),
+ Err(_) => return Err(IonError::Unterminated),
+ }
}
+ Ok(self.previous_status)
}
/// Obtains a variable, returning an empty string if it does not exist.
diff --git a/src/lib/shell/variables/mod.rs b/src/lib/shell/variables/mod.rs
index 6aaf04b1..dbe659c7 100644
--- a/src/lib/shell/variables/mod.rs
+++ b/src/lib/shell/variables/mod.rs
@@ -301,78 +301,46 @@ impl Variables {
}
pub(crate) fn tilde_expansion(&self, word: &str, dir_stack: &DirectoryStack) -> Option<String> {
- let mut chars = word.char_indices();
+ // Only if the first character is a tilde character will we perform expansions
+ if !word.starts_with('~') {
+ return None;
+ }
- let tilde_prefix;
- let remainder;
+ let mut parts = word[1..].splitn(2, |c| c == '/' || c == '$');
- loop {
- if let Some((ind, c)) = chars.next() {
- if c == '/' || c == '$' {
- tilde_prefix = &word[1..ind];
- remainder = &word[ind..];
- break;
- }
- } else {
- tilde_prefix = &word[1..];
- remainder = "";
- break;
- }
- }
+ match parts.next().unwrap() {
+ "" => sys_env::home_dir()
+ .map(|home| home.to_string_lossy().to_string() + parts.next().unwrap_or("")),
- match tilde_prefix {
- "" => {
- if let Some(home) = sys_env::home_dir() {
- return Some(home.to_string_lossy().to_string() + remainder);
- }
- }
- "+" => {
- return Some(match env::var("PWD") {
- Ok(var) => var + remainder,
- _ => ["?", remainder].concat(),
- });
- }
- "-" => {
- if let Some(oldpwd) = self.get::<types::Str>("OLDPWD") {
- return Some(oldpwd.to_string() + remainder);
- }
- }
- _ => {
- let neg;
- let tilde_num;
+ "+" => Some(
+ env::var("PWD").unwrap_or_else(|_| "?".to_string()) + parts.next().unwrap_or(""),
+ ),
+
+ "-" => self
+ .get::<types::Str>("OLDPWD")
+ .map(|oldpwd| oldpwd.to_string() + parts.next().unwrap_or("")),
- if tilde_prefix.starts_with('+') {
- tilde_num = &tilde_prefix[1..];
- neg = false;
+ tilde_prefix => {
+ let (neg, tilde_num) = if tilde_prefix.starts_with('+') {
+ (false, &tilde_prefix[1..])
} else if tilde_prefix.starts_with('-') {
- tilde_num = &tilde_prefix[1..];
- neg = true;
+ (true, &tilde_prefix[1..])
} else {
- tilde_num = tilde_prefix;
- neg = false;
- }
+ (false, tilde_prefix)
+ };
match tilde_num.parse() {
- Ok(num) => {
- let res = if neg {
- dir_stack.dir_from_top(num)
- } else {
- dir_stack.dir_from_bottom(num)
- };
-
- if let Some(path) = res {
- return Some(path.to_str().unwrap().to_string());
- }
- }
- Err(_) => {
- if let Some(home) = self_sys::get_user_home(tilde_prefix) {
- return Some(home + remainder);
- }
+ Ok(num) => if neg {
+ dir_stack.dir_from_top(num)
+ } else {
+ dir_stack.dir_from_bottom(num)
}
+ .map(|path| path.to_str().unwrap().to_string()),
+ Err(_) => self_sys::get_user_home(tilde_prefix)
+ .map(|home| home + parts.next().unwrap_or("")),
}
}
}
- None
}
pub(crate) fn is_valid_variable_name(name: &str) -> bool {
--
GitLab