From ceaed5c543e193c918638fd8bf201cb365e19286 Mon Sep 17 00:00:00 2001 From: Jeremy Wall Date: Tue, 19 Feb 2019 16:36:19 -0600 Subject: [PATCH] REFACTOR: No longer require PositionedItem for Val::Tuples. --- src/build/ir.rs | 12 +++++++----- src/build/mod.rs | 39 ++++++++++++++++++--------------------- src/build/scope.rs | 9 +++------ src/convert/env.rs | 9 ++------- src/convert/exec.rs | 27 ++++++++++++++------------- src/convert/flags.rs | 6 +++--- src/convert/json.rs | 13 +++---------- src/convert/toml.rs | 5 ++--- src/convert/xml.rs | 32 +++++++++++++++----------------- src/convert/yaml.rs | 16 +++------------- 10 files changed, 70 insertions(+), 98 deletions(-) diff --git a/src/build/ir.rs b/src/build/ir.rs index 17d183d..f47abd1 100644 --- a/src/build/ir.rs +++ b/src/build/ir.rs @@ -18,7 +18,8 @@ pub enum Val { Float(f64), Str(String), List(Vec>), - Tuple(Vec<(PositionedItem, Rc)>), + // TODO(jwall): Remove the need for PositionedItem here + Tuple(Vec<(String, Rc)>), Env(Vec<(String, String)>), Func(FuncDef), Module(ModuleDef), @@ -86,12 +87,12 @@ impl Val { } else { for (i, lv) in ldef.iter().enumerate() { let field_target = &rdef[i]; - if lv.0.val != field_target.0.val { + if lv.0 != field_target.0 { // field name equality return Ok(false); } else { // field value equality. - if !lv.1.equal(field_target.1.as_ref(), lv.0.pos.clone())? { + if !lv.1.equal(field_target.1.as_ref(), pos.clone())? { return Ok(false); } } @@ -99,6 +100,7 @@ impl Val { Ok(true) } } + // FIXME(jwall): Maybe we don't require positions here? (&Val::Func(_), &Val::Func(_)) => Err(error::BuildError::new( "Func are not comparable", error::ErrorType::TypeFail, @@ -121,7 +123,7 @@ impl Val { } /// Returns the fields if this Val is a tuple. None otherwise. - pub fn get_fields(&self) -> Option<&Vec<(PositionedItem, Rc)>> { + pub fn get_fields(&self) -> Option<&Vec<(String, Rc)>> { if let &Val::Tuple(ref fs) = self { Some(fs) } else { @@ -227,7 +229,7 @@ impl Display for Val { &Val::Tuple(ref def) => { write!(f, "{{\n")?; for v in def.iter() { - write!(f, "\t{} = {},\n", v.0.val, v.1)?; + write!(f, "\t{} = {},\n", v.0, v.1)?; } write!(f, "}}") } diff --git a/src/build/mod.rs b/src/build/mod.rs index ab6e05e..adf27eb 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -254,10 +254,10 @@ impl<'a> FileBuilder<'a> { fields: &Vec<(Token, Expression)>, scope: &Scope, ) -> Result, Box> { - let mut new_fields = Vec::<(PositionedItem, Rc)>::new(); + let mut new_fields = Vec::<(String, Rc)>::new(); for &(ref name, ref expr) in fields.iter() { let val = self.eval_expr(expr, scope)?; - new_fields.push((name.into(), val)); + new_fields.push((name.fragment.clone(), val)); } Ok(Rc::new(Val::Tuple(new_fields))) } @@ -1014,18 +1014,22 @@ impl<'a> FileBuilder<'a> { } fn get_outputs_as_val(&mut self) -> Rc { - let fields: Vec<(PositionedItem, Rc)> = - self.scope.build_output.drain().collect(); + let fields: Vec<(String, Rc)> = self + .scope + .build_output + .drain() + .map(|v| (v.0.val, v.1)) + .collect(); Rc::new(Val::Tuple(fields)) } fn copy_from_base( &self, - src_fields: &Vec<(PositionedItem, Rc)>, + src_fields: &Vec<(String, Rc)>, overrides: &Vec<(Token, Expression)>, scope: &Scope, ) -> Result, Box> { - let mut m = HashMap::, (i32, Rc)>::new(); + let mut m = HashMap::)>::new(); // loop through fields and build up a hashmap let mut count = 0; for &(ref key, ref val) in src_fields.iter() { @@ -1038,16 +1042,16 @@ impl<'a> FileBuilder<'a> { "Duplicate \ field: {} in \ tuple", - key.val + key ), error::ErrorType::TypeFail, - key.pos.clone(), + Position::new(0, 0, 0), ))); } } for &(ref key, ref val) in overrides.iter() { let expr_result = self.eval_expr(val, scope)?; - match m.entry(key.into()) { + match m.entry(key.fragment.clone()) { // brand new field here. Entry::Vacant(v) => { v.insert((count, expr_result)); @@ -1077,7 +1081,7 @@ impl<'a> FileBuilder<'a> { } }; } - let mut new_fields: Vec<(PositionedItem, (i32, Rc))> = m.drain().collect(); + let mut new_fields: Vec<(String, (i32, Rc))> = m.drain().collect(); // We want to maintain our order for the fields to make comparing tuples // easier in later code. So we sort by the field order before constructing a new tuple. new_fields.sort_by(|a, b| { @@ -1312,13 +1316,13 @@ impl<'a> FileBuilder<'a> { fn eval_functional_tuple_processing( &self, - fs: &Vec<(PositionedItem, Rc)>, + fs: &Vec<(String, Rc)>, def: &FuncDef, typ: ProcessingOpType, ) -> Result, Box> { let mut out = Vec::new(); for &(ref name, ref val) in fs { - let argvals = vec![Rc::new(Val::Str(name.val.clone())), val.clone()]; + let argvals = vec![Rc::new(Val::Str(name.clone())), val.clone()]; let result = def.eval(self, argvals)?; match typ { ProcessingOpType::Map => { @@ -1338,10 +1342,7 @@ impl<'a> FileBuilder<'a> { def.pos.clone(), ))); }; - out.push(( - PositionedItem::new(new_name, name.pos.clone()), - fs[1].clone(), - )); + out.push((new_name, fs[1].clone())); } else { return Err(Box::new(error::BuildError::new( format!( @@ -1402,11 +1403,7 @@ impl<'a> FileBuilder<'a> { } &Val::Tuple(ref fs) => { for &(ref name, ref val) in fs.iter() { - let argvals = vec![ - acc.clone(), - Rc::new(Val::Str(name.val.clone())), - val.clone(), - ]; + let argvals = vec![acc.clone(), Rc::new(Val::Str(name.clone())), val.clone()]; let result = funcdef.eval(self, argvals)?; acc = result; } diff --git a/src/build/scope.rs b/src/build/scope.rs index ddd5356..eba0f9b 100644 --- a/src/build/scope.rs +++ b/src/build/scope.rs @@ -10,12 +10,9 @@ use crate::ast::PositionedItem; use crate::build::ir::Val; use crate::error; -pub fn find_in_fieldlist( - target: &str, - fs: &Vec<(PositionedItem, Rc)>, -) -> Option> { +pub fn find_in_fieldlist(target: &str, fs: &Vec<(String, Rc)>) -> Option> { for (key, val) in fs.iter().cloned() { - if target == &key.val { + if target == &key { return Some(val.clone()); } } @@ -187,7 +184,7 @@ impl Scope { fn lookup_in_tuple( pos: &Position, field: &str, - fs: &Vec<(PositionedItem, Rc)>, + fs: &Vec<(String, Rc)>, ) -> Result, Box> { if let Some(vv) = find_in_fieldlist(&field, fs) { Ok(vv) diff --git a/src/convert/env.rs b/src/convert/env.rs index 9fde39e..bf5218d 100644 --- a/src/convert/env.rs +++ b/src/convert/env.rs @@ -16,7 +16,6 @@ use std::io::Write; use std::rc::Rc; -use crate::ast::PositionedItem; use crate::build::Val; use crate::convert::traits::{ConvertResult, Converter}; @@ -29,11 +28,7 @@ impl EnvConverter { EnvConverter {} } - fn convert_tuple( - &self, - flds: &Vec<(PositionedItem, Rc)>, - w: &mut Write, - ) -> ConvertResult { + fn convert_tuple(&self, flds: &Vec<(String, Rc)>, w: &mut Write) -> ConvertResult { for &(ref name, ref val) in flds.iter() { if val.is_tuple() { eprintln!("Skipping embedded tuple..."); @@ -43,7 +38,7 @@ impl EnvConverter { eprintln!("Skipping empty variable: {}", name); return Ok(()); } - write!(w, "{}=", name.val)?; + write!(w, "{}=", name)?; self.write(&val, w)?; } Ok(()) diff --git a/src/convert/exec.rs b/src/convert/exec.rs index b24a8b8..4c8e79f 100644 --- a/src/convert/exec.rs +++ b/src/convert/exec.rs @@ -17,7 +17,7 @@ use std; use std::io::{Cursor, Write}; use std::rc::Rc; -use crate::ast::{Position, PositionedItem}; +use crate::ast::Position; use crate::build::Val; use crate::build::Val::Tuple; use crate::convert; @@ -49,17 +49,18 @@ impl ExecConverter { Position::new(0, 0, 0), ))); } - let mut env: Option<&Vec<(PositionedItem, Rc)>> = None; + let mut env: Option<&Vec<(String, Rc)>> = None; let mut command: Option<&str> = None; let mut args: Option<&Vec>> = None; for &(ref name, ref val) in fields.iter() { + // FIXME(jwall): BuildError should not require a Position. // We require a command field in our exec tuple. - if name.val == "command" { + if name == "command" { if command.is_some() { return Err(Box::new(BuildError::new( "There can only be one command field in an exec tuple", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } if let &Val::Str(ref s) = val.as_ref() { @@ -69,17 +70,17 @@ impl ExecConverter { return Err(Box::new(BuildError::new( "The command field of an exec tuple must be a string", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } // We optionally allow an env field in our exec tuple. - if name.val == "env" { + if name == "env" { if let &Val::Tuple(ref l) = val.as_ref() { if env.is_some() { return Err(Box::new(BuildError::new( "There can only be one env field in an exec tuple", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } env = Some(l); @@ -88,17 +89,17 @@ impl ExecConverter { return Err(Box::new(BuildError::new( "The env field of an exec tuple must be a list", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } // We optionally allow an args field in our exec tuple. - if name.val == "args" { + if name == "args" { if let &Val::List(ref l) = val.as_ref() { if args.is_some() { return Err(Box::new(BuildError::new( "There can only be one args field of an exec tuple", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } args = Some(l); @@ -107,7 +108,7 @@ impl ExecConverter { return Err(Box::new(BuildError::new( "The args field of an exec tuple must be a list", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } } @@ -130,13 +131,13 @@ impl ExecConverter { for &(ref name, ref v) in env_list.iter() { // We only allow string fields in our env tuple. if let &Val::Str(ref s) = v.as_ref() { - write!(script, "{}=\"{}\"\n", name.val, s)?; + write!(script, "{}=\"{}\"\n", name, s)?; continue; } return Err(Box::new(BuildError::new( "The env fields of an exec tuple must contain only string values", ErrorType::TypeFail, - name.pos.clone(), + Position::new(0, 0, 0), ))); } } diff --git a/src/convert/flags.rs b/src/convert/flags.rs index c245d55..fc87884 100644 --- a/src/convert/flags.rs +++ b/src/convert/flags.rs @@ -84,7 +84,7 @@ impl FlagConverter { &Val::Tuple(ref flds) => { for &(ref name, ref val) in flds.iter() { if let &Val::Empty = val.as_ref() { - self.write_flag_name(pfx, &name.val, w)?; + self.write_flag_name(pfx, name, w)?; continue; } match val.as_ref() { @@ -93,10 +93,10 @@ impl FlagConverter { self.write(&new_pfx, val, w)?; } &Val::List(ref def) => { - self.write_list_flag(pfx, &name.val, def, w)?; + self.write_list_flag(pfx, name, def, w)?; } _ => { - self.write_flag_name(pfx, &name.val, w)?; + self.write_flag_name(pfx, name, w)?; self.write(pfx, &val, w)?; } } diff --git a/src/convert/json.rs b/src/convert/json.rs index 89ccaa6..04d8d92 100644 --- a/src/convert/json.rs +++ b/src/convert/json.rs @@ -15,7 +15,6 @@ use std::rc::Rc; use serde_json; -use crate::ast; use crate::build::Val; use crate::convert::traits::{ConvertResult, Converter, ImportResult, Importer}; @@ -35,13 +34,10 @@ impl JsonConverter { Ok(serde_json::Value::Array(v)) } - fn convert_tuple( - &self, - items: &Vec<(ast::PositionedItem, Rc)>, - ) -> std::io::Result { + fn convert_tuple(&self, items: &Vec<(String, Rc)>) -> std::io::Result { let mut mp = serde_json::Map::new(); for &(ref k, ref v) in items.iter() { - mp.entry(k.val.clone()).or_insert(self.convert_value(v)?); + mp.entry(k.clone()).or_insert(self.convert_value(v)?); } Ok(serde_json::Value::Object(mp)) } @@ -113,10 +109,7 @@ impl JsonConverter { serde_json::Value::Object(m) => { let mut fs = Vec::with_capacity(m.len()); for (key, value) in m { - fs.push(( - ast::PositionedItem::new(key.to_string(), ast::Position::new(0, 0, 0)), - Rc::new(self.convert_json_val(value)?), - )); + fs.push((key.to_string(), Rc::new(self.convert_json_val(value)?))); } Val::Tuple(fs) } diff --git a/src/convert/toml.rs b/src/convert/toml.rs index 2b34e63..70796b6 100644 --- a/src/convert/toml.rs +++ b/src/convert/toml.rs @@ -20,7 +20,6 @@ use std::rc::Rc; use simple_error::SimpleError; use toml; -use crate::ast; use crate::build::Val; use crate::convert::traits::{ConvertResult, Converter}; @@ -41,10 +40,10 @@ impl TomlConverter { Ok(toml::Value::Array(v)) } - fn convert_tuple(&self, items: &Vec<(ast::PositionedItem, Rc)>) -> Result { + fn convert_tuple(&self, items: &Vec<(String, Rc)>) -> Result { let mut mp = toml::value::Table::new(); for &(ref k, ref v) in items.iter() { - mp.entry(k.val.clone()).or_insert(self.convert_value(v)?); + mp.entry(k.clone()).or_insert(self.convert_value(v)?); } Ok(toml::Value::Table(mp)) } diff --git a/src/convert/xml.rs b/src/convert/xml.rs index 1e10bc3..3732c0f 100644 --- a/src/convert/xml.rs +++ b/src/convert/xml.rs @@ -18,7 +18,7 @@ use std::io::Write; use std::rc::Rc; use super::traits::{ConvertResult, Converter}; -use crate::ast::{Position, PositionedItem}; +use crate::ast::Position; use crate::build::Val; use crate::error::BuildError; use crate::error::ErrorType; @@ -43,9 +43,7 @@ impl XmlConverter { } } - fn get_tuple_val( - v: &Val, - ) -> std::result::Result<&Vec<(PositionedItem, Rc)>, Box> { + fn get_tuple_val(v: &Val) -> std::result::Result<&Vec<(String, Rc)>, Box> { if let Val::Tuple(ref fs) = v { Ok(fs) } else { @@ -73,15 +71,15 @@ impl XmlConverter { // First we determine if this is a tag or text node if let Val::Tuple(ref fs) = v { let mut name: Option<&str> = None; - let mut attrs: Option<&Vec<(PositionedItem, Rc)>> = None; + let mut attrs: Option<&Vec<(String, Rc)>> = None; let mut children: Option<&Vec>> = None; let mut text: Option<&str> = None; let mut ns: Option<(&str, &str)> = None; for (ref field, ref val) in fs.iter() { - if field.val == "name" { + if field == "name" { name = Some(Self::get_str_val(val.as_ref())?); } - if field.val == "ns" { + if field == "ns" { if let Val::Tuple(ref fs) = val.as_ref() { let mut prefix = ""; let mut uri = ""; @@ -89,10 +87,10 @@ impl XmlConverter { if val.is_empty() { continue; } - if name.val == "uri" { + if name == "uri" { uri = Self::get_str_val(val.as_ref())?; } - if name.val == "prefix" { + if name == "prefix" { prefix = Self::get_str_val(val.as_ref())?; } } @@ -103,19 +101,19 @@ impl XmlConverter { ns = Some(("", s)); } } - if field.val == "attrs" { + if field == "attrs" { // This should be a tuple. if !val.is_empty() { attrs = Some(Self::get_tuple_val(val.as_ref())?); } } - if field.val == "children" { + if field == "children" { // This should be a list of tuples. if !val.is_empty() { children = Some(Self::get_list_val(val.as_ref())?); } } - if field.val == "text" { + if field == "text" { if !val.is_empty() { text = Some(Self::get_str_val(val.as_ref())?); } @@ -135,7 +133,7 @@ impl XmlConverter { if val.is_empty() { continue; } - start = start.attr(name.val.as_ref(), Self::get_str_val(val.as_ref())?); + start = start.attr(name.as_ref(), Self::get_str_val(val.as_ref())?); } } if let Some((prefix, uri)) = ns { @@ -175,19 +173,19 @@ impl XmlConverter { let mut standalone: Option = None; let mut root: Option> = None; for &(ref name, ref val) in fs.iter() { - if name.val == "version" { + if name == "version" { version = Some(Self::get_str_val(val)?); } - if name.val == "encoding" { + if name == "encoding" { encoding = Some(Self::get_str_val(val)?); } - if name.val == "standalone" { + if name == "standalone" { standalone = match val.as_ref() { Val::Boolean(b) => Some(*b), _ => None, }; } - if name.val == "root" { + if name == "root" { root = Some(val.clone()); } } diff --git a/src/convert/yaml.rs b/src/convert/yaml.rs index 03a8ad8..bc6ff3f 100644 --- a/src/convert/yaml.rs +++ b/src/convert/yaml.rs @@ -7,7 +7,6 @@ use std::result::Result; use serde_yaml; use super::traits::{ConvertResult, Converter, ImportResult, Importer}; -use crate::ast; use crate::build::Val; pub struct YamlConverter {} @@ -36,16 +35,10 @@ impl YamlConverter { Ok(serde_yaml::Value::Mapping(mp)) } - fn convert_tuple( - &self, - items: &Vec<(ast::PositionedItem, Rc)>, - ) -> std::io::Result { + fn convert_tuple(&self, items: &Vec<(String, Rc)>) -> std::io::Result { let mut mapping = serde_yaml::Mapping::new(); for &(ref k, ref v) in items.iter() { - mapping.insert( - serde_yaml::Value::String(k.val.clone()), - self.convert_value(v)?, - ); + mapping.insert(serde_yaml::Value::String(k.clone()), self.convert_value(v)?); } Ok(serde_yaml::Value::Mapping(mapping)) } @@ -113,10 +106,7 @@ impl YamlConverter { } }; eprintln!("yaml key is: {}", key); - fs.push(( - ast::PositionedItem::new(key, ast::Position::new(0, 0, 0)), - Rc::new(self.convert_json_val(value)?), - )); + fs.push((key, Rc::new(self.convert_json_val(value)?))); } Val::Tuple(fs) }