Mike Gerwitz

Activist for User Freedom

aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoseph Frazer <joseph.frazer@ryansg.com>2020-03-09 13:41:26 -0400
committerJoseph Frazer <joseph.frazer@ryansg.com>2020-03-09 13:41:26 -0400
commit2434e138b830edd05e45d07e6f90d157e736623f (patch)
tree03d05259389fe18809c65273cc94b8d7b42f2c93
parentbfea768f893dfd6fdddc9f620dac95757770b682 (diff)
parentb5f6a082dd4bb539296ff7d3e5398304fa2d62e2 (diff)
downloadtame-2434e138b830edd05e45d07e6f90d157e736623f.tar.gz
tame-2434e138b830edd05e45d07e6f90d157e736623f.tar.bz2
tame-2434e138b830edd05e45d07e6f90d157e736623f.zip
[DEV-7134] Propagate errors
Merge branch 'jira-7134' * jira-7134: [DEV-7134] Remove unnecessary node replacement [DEV-7134] Propagate errors from the writer [DEV-7134] Propagate sorting errors [DEV-7134] Propagate errors setting fragments [DEV-7134] Pass read event errors up the stack [DEV-7134] Return error for XmloEvent::SymDecl [DEV-7134] Add alias for LoadResult [DEV-7134] Remove unwrap so we can bubble up error messages [DEV-7134] Escalate the error from finding the absolute path
-rw-r--r--tamer/src/ir/asg/base.rs111
-rw-r--r--tamer/src/ir/asg/graph.rs5
-rw-r--r--tamer/src/ir/asg/mod.rs2
-rw-r--r--tamer/src/ld/poc.rs86
-rw-r--r--tamer/src/obj/xmle/writer/writer.rs23
-rw-r--r--tamer/src/obj/xmlo/reader.rs5
-rw-r--r--tamer/tests/tameld.rs1
7 files changed, 171 insertions, 62 deletions
diff --git a/tamer/src/ir/asg/base.rs b/tamer/src/ir/asg/base.rs
index ba0bd21..5cedf83 100644
--- a/tamer/src/ir/asg/base.rs
+++ b/tamer/src/ir/asg/base.rs
@@ -213,15 +213,47 @@ where
Object::Ident(sym, kind, src) => {
Ok(Object::IdentFragment(sym, kind, src, text))
}
- _ => {
- let err = Err(AsgError::BadFragmentDest(format!(
- "identifier is not a Object::Ident): {:?}",
- ty,
- )));
-
- node.replace(ty);
- err
+ Object::IdentFragment(_, IdentKind::MapHead, _, _) => Ok(ty),
+ Object::IdentFragment(_, IdentKind::MapTail, _, _) => Ok(ty),
+ Object::IdentFragment(_, IdentKind::RetMapHead, _, _) => Ok(ty),
+ Object::IdentFragment(_, IdentKind::RetMapTail, _, _) => Ok(ty),
+ // TODO remove these ignores when fixed
+ Object::IdentFragment(
+ sym,
+ IdentKind::Map,
+ Source {
+ virtual_: true,
+ override_: true,
+ ..
+ },
+ _,
+ ) => {
+ eprintln!(
+ "ignoring virtual and overridden map object: {}",
+ sym
+ );
+ Ok(ty)
}
+ Object::IdentFragment(
+ sym,
+ IdentKind::RetMap,
+ Source {
+ virtual_: true,
+ override_: true,
+ ..
+ },
+ _,
+ ) => {
+ eprintln!(
+ "ignoring virtual and overridden retmap object: {}",
+ sym
+ );
+ Ok(ty)
+ }
+ _ => Err(AsgError::BadFragmentDest(format!(
+ "identifier is not a Object::Ident): {:?}",
+ ty,
+ ))),
}?;
node.replace(result);
@@ -536,6 +568,58 @@ mod test {
Ok(())
}
+ fn add_ident_kind_ignores(
+ given: IdentKind,
+ expected: IdentKind,
+ ) -> AsgResult<()> {
+ let mut sut = Sut::with_capacity(0, 0);
+
+ let sym = Symbol::new_dummy(SymbolIndex::from_u32(1), "tofrag");
+ let src = Source {
+ generated: true,
+ ..Default::default()
+ };
+
+ let node = sut.declare(&sym, given, src.clone())?;
+
+ let fragment = "a fragment".to_string();
+ let node_with_frag = sut.set_fragment(node, fragment.clone())?;
+
+ // Attaching a fragment should _replace_ the node, not create a
+ // new one
+ assert_eq!(
+ node, node_with_frag,
+ "fragment node does not match original node"
+ );
+
+ assert_eq!(
+ Some(&Object::IdentFragment(&sym, expected, src, fragment)),
+ sut.get(node)
+ );
+
+ Ok(())
+ }
+
+ #[test]
+ fn add_fragment_to_ident_map_head() -> AsgResult<()> {
+ add_ident_kind_ignores(IdentKind::MapHead, IdentKind::MapHead)
+ }
+
+ #[test]
+ fn add_fragment_to_ident_map_tail() -> AsgResult<()> {
+ add_ident_kind_ignores(IdentKind::MapTail, IdentKind::MapTail)
+ }
+
+ #[test]
+ fn add_fragment_to_ident_retmap_head() -> AsgResult<()> {
+ add_ident_kind_ignores(IdentKind::RetMapHead, IdentKind::RetMapHead)
+ }
+
+ #[test]
+ fn add_fragment_to_ident_retmap_tail() -> AsgResult<()> {
+ add_ident_kind_ignores(IdentKind::RetMapTail, IdentKind::RetMapTail)
+ }
+
#[test]
fn add_fragment_to_fragment_fails() -> AsgResult<()> {
let mut sut = Sut::with_capacity(0, 0);
@@ -556,17 +640,6 @@ mod test {
_ => panic!("expected AsgError::BadFragmentDest: {:?}", err),
}
- // Make sure we didn't leave the node in an inconsistent state
- assert_eq!(
- Some(&Object::IdentFragment(
- &sym,
- IdentKind::Meta,
- Default::default(),
- fragment
- )),
- sut.get(node)
- );
-
Ok(())
}
diff --git a/tamer/src/ir/asg/graph.rs b/tamer/src/ir/asg/graph.rs
index e1e38f7..1f56e6c 100644
--- a/tamer/src/ir/asg/graph.rs
+++ b/tamer/src/ir/asg/graph.rs
@@ -214,6 +214,8 @@ pub enum AsgError {
///
/// See [`Asg::set_fragment`] for more information.
BadFragmentDest(String),
+ /// The node was not expected in the current context
+ UnexpectedNode(String),
}
impl std::fmt::Display for AsgError {
@@ -222,6 +224,9 @@ impl std::fmt::Display for AsgError {
Self::BadFragmentDest(msg) => {
write!(fmt, "bad fragment destination: {}", msg)
}
+ Self::UnexpectedNode(msg) => {
+ write!(fmt, "unexpected node: {}", msg)
+ }
}
}
}
diff --git a/tamer/src/ir/asg/mod.rs b/tamer/src/ir/asg/mod.rs
index e5a397b..8905768 100644
--- a/tamer/src/ir/asg/mod.rs
+++ b/tamer/src/ir/asg/mod.rs
@@ -186,7 +186,7 @@ mod graph;
mod ident;
mod object;
-pub use graph::{Asg, AsgResult, ObjectRef};
+pub use graph::{Asg, AsgError, AsgResult, ObjectRef};
pub use ident::{Dim, IdentKind};
pub use object::{FragmentText, Object, Source};
diff --git a/tamer/src/ld/poc.rs b/tamer/src/ld/poc.rs
index e82c19f..52f4366 100644
--- a/tamer/src/ld/poc.rs
+++ b/tamer/src/ld/poc.rs
@@ -21,8 +21,9 @@
//! banished to its own file to try to make that more clear.
use crate::global;
-use crate::ir::asg::IdentKind;
-use crate::ir::asg::{Asg, DefaultAsg, Object, ObjectRef, Source};
+use crate::ir::asg::{
+ Asg, AsgError, DefaultAsg, IdentKind, Object, ObjectRef, Source,
+};
use crate::obj::xmle::writer::{Sections, XmleWriter};
use crate::obj::xmlo::reader::{XmloError, XmloEvent, XmloReader};
use crate::sym::{DefaultInterner, Interner, Symbol};
@@ -36,6 +37,9 @@ use std::io::BufReader;
type LinkerAsg<'i> = DefaultAsg<'i, global::ProgIdentSize>;
type LinkerObjectRef = ObjectRef<global::ProgIdentSize>;
+type LoadResult<'i> =
+ Result<Option<(Option<&'i Symbol<'i>>, Option<String>)>, Box<dyn Error>>;
+
pub fn main(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
let mut pkgs_seen: FxHashSet<String> = Default::default();
let mut fragments: FxHashMap<&str, String> = Default::default();
@@ -43,7 +47,7 @@ pub fn main(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
let mut roots = Vec::new();
let interner = DefaultInterner::new();
- let abs_path = fs::canonicalize(package_path).unwrap();
+ let abs_path = fs::canonicalize(package_path)?;
println!("WARNING: This is proof-of-concept; do not use!");
@@ -74,7 +78,7 @@ pub fn main(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
.filter_map(|sym| depgraph.lookup(sym)),
);
- let mut sorted = sort_deps(&depgraph, &roots);
+ let mut sorted = sort_deps(&depgraph, &roots)?;
//println!("Sorted ({}): {:?}", sorted.len(), sorted);
@@ -97,7 +101,7 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
depgraph: &mut LinkerAsg<'i>,
interner: &'i I,
roots: &mut Vec<LinkerObjectRef>,
-) -> Result<Option<(Option<&'i Symbol<'i>>, Option<String>)>, Box<dyn Error>> {
+) -> LoadResult<'i> {
let path = fs::canonicalize(path_str)?;
let path_str = path.to_str().unwrap();
@@ -164,7 +168,6 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
src.pkg_name = None;
}
- // TODO: should probably track these down in the XSLT linker...
match kind {
Ok(kindval) => {
// TODO: inefficient
@@ -179,22 +182,23 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
roots.push(node);
}
}
- Err(e) => println!("{:?}; skipping...", e),
+ Err(e) => return Err(e.into()),
};
}
}
Ok(XmloEvent::Fragment(sym, text)) => {
- let result = depgraph.set_fragment(
- depgraph.lookup(sym).unwrap_or_else(|| {
- panic!("missing symbol for fragment: {}", sym)
- }),
- text,
- );
-
- match result {
- Ok(_) => (),
- Err(e) => println!("{:?}; skipping...", e),
+ match depgraph.lookup(sym) {
+ Some(frag) => match depgraph.set_fragment(frag, text) {
+ Ok(_) => (),
+ Err(e) => return Err(e.into()),
+ },
+ None => {
+ return Err(XmloError::MissingFragment(String::from(
+ "missing fragment",
+ ))
+ .into());
+ }
};
}
@@ -202,11 +206,7 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
// header (symtable, sym-deps, fragments)
Ok(XmloEvent::Eoh) => break,
- Err(err @ XmloError::UnassociatedFragment) => {
- println!("{:?}; skipping...", err);
- }
-
- err @ Err(_) => err.map(|_| ())?,
+ Err(e) => return Err(e.into()),
}
}
@@ -225,7 +225,7 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
path_buf.set_extension("xmlo");
// println!("Trying {:?}", path_buf);
- let path_abs = path_buf.canonicalize().unwrap();
+ let path_abs = path_buf.canonicalize()?;
let path = path_abs.to_str().unwrap();
load_xmlo(path, pkgs_seen, fragments, depgraph, interner, roots)?;
@@ -241,7 +241,7 @@ fn load_xmlo<'a, 'i, I: Interner<'i>>(
fn sort_deps<'a, 'i>(
depgraph: &'a LinkerAsg<'i>,
roots: &Vec<LinkerObjectRef>,
-) -> Sections<'a, 'i> {
+) -> Result<Sections<'a, 'i>, Box<dyn Error>> {
// @type=meta, @preproc:elig-class-yields
// @type={ret}map{,:head,:tail}
@@ -261,7 +261,7 @@ fn sort_deps<'a, 'i>(
// TODO: can we encapsulate NodeIndex?
while let Some(index) = dfs.next(&depgraph) {
- let ident = depgraph.get(index).unwrap();
+ let ident = depgraph.get(index).expect("missing node");
match ident {
Object::Ident(_, kind, _)
@@ -279,25 +279,29 @@ fn sort_deps<'a, 'i>(
| IdentKind::RetMapTail => deps.retmap.push_body(ident),
_ => deps.rater.push_body(ident),
},
- _ => panic!("unexpected node: {:?}", ident),
+ _ => {
+ return Err(
+ AsgError::UnexpectedNode(format!("{:?}", ident)).into()
+ )
+ }
}
}
- deps
+ Ok(deps)
}
fn get_interner_value<'a, 'i, I: Interner<'i>>(
depgraph: &'a LinkerAsg<'i>,
interner: &'i I,
name: &str,
-) -> &'a Object<'i> {
- depgraph
- .get(
- depgraph
- .lookup(interner.intern(name))
- .unwrap_or_else(|| panic!("missing identifier: {}", name)),
- )
- .expect("Could not get interner value")
+) -> Result<&'a Object<'i>, Box<dyn Error>> {
+ match depgraph.lookup(interner.intern(name)) {
+ Some(frag) => match depgraph.get(frag) {
+ Some(result) => Ok(result),
+ None => Err(XmloError::MissingFragment(String::from(name)).into()),
+ },
+ None => Err(XmloError::MissingFragment(String::from(name)).into()),
+ }
}
fn output_xmle<'a, 'i, I: Interner<'i>>(
@@ -313,12 +317,12 @@ fn output_xmle<'a, 'i, I: Interner<'i>>(
depgraph,
interner,
&String::from(":map:___head"),
- ));
+ )?);
sorted.map.push_tail(get_interner_value(
depgraph,
interner,
&String::from(":map:___tail"),
- ));
+ )?);
}
if !sorted.retmap.is_empty() {
@@ -326,19 +330,17 @@ fn output_xmle<'a, 'i, I: Interner<'i>>(
depgraph,
interner,
&String::from(":retmap:___head"),
- ));
+ )?);
sorted.retmap.push_tail(get_interner_value(
depgraph,
interner,
&String::from(":retmap:___tail"),
- ));
+ )?);
}
let file = fs::File::create(output)?;
let mut xmle_writer = XmleWriter::new(file);
- xmle_writer
- .write(&sorted, name, &relroot)
- .expect("Could not write xmle output");
+ xmle_writer.write(&sorted, name, &relroot)?;
Ok(())
}
diff --git a/tamer/src/obj/xmle/writer/writer.rs b/tamer/src/obj/xmle/writer/writer.rs
index 76f21a8..7c592cf 100644
--- a/tamer/src/obj/xmle/writer/writer.rs
+++ b/tamer/src/obj/xmle/writer/writer.rs
@@ -187,12 +187,35 @@ impl<'a, 'i> Sections<'a, 'i> {
/// Error implementations for the writer
#[derive(Debug)]
pub enum WriterError {
+ /// Propagated IO error
Io(IoError),
+ /// Propagated UTF8 error
Utf8(Utf8Error),
+ /// Propagated XML error
XmlError(XmlError),
+ /// Something other than a fragment was given when a fragment was expected
ExpectedFragment(String),
}
+impl std::fmt::Display for WriterError {
+ fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+ match self {
+ Self::Io(inner) => inner.fmt(fmt),
+ Self::Utf8(inner) => inner.fmt(fmt),
+ Self::XmlError(inner) => inner.fmt(fmt),
+ Self::ExpectedFragment(msg) => {
+ write!(fmt, "expected fragment: {}", msg)
+ }
+ }
+ }
+}
+
+impl std::error::Error for WriterError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ None
+ }
+}
+
impl From<IoError> for WriterError {
fn from(err: IoError) -> Self {
WriterError::Io(err)
diff --git a/tamer/src/obj/xmlo/reader.rs b/tamer/src/obj/xmlo/reader.rs
index 55723ed..ded13c0 100644
--- a/tamer/src/obj/xmlo/reader.rs
+++ b/tamer/src/obj/xmlo/reader.rs
@@ -764,6 +764,8 @@ pub enum XmloError {
UnassociatedFragment,
/// A `preproc:fragment` element was found, but is missing `text()`.
MissingFragmentText(String),
+ /// A `preproc:fragment` element was not found
+ MissingFragment(String),
}
impl From<XmlError> for XmloError {
@@ -811,6 +813,9 @@ impl Display for XmloError {
"fragment found, but missing text for symbol `{}`",
symname,
),
+ XmloError::MissingFragment(symname) => {
+ write!(fmt, "fragment not found `{}`", symname,)
+ }
}
}
}
diff --git a/tamer/tests/tameld.rs b/tamer/tests/tameld.rs
index 98d1974..30b2fea 100644
--- a/tamer/tests/tameld.rs
+++ b/tamer/tests/tameld.rs
@@ -61,6 +61,7 @@ fn link_input_file_does_not_exist() -> Result<(), Box<dyn std::error::Error>> {
cmd.arg("-o").arg("tests/data/test-output.xmle");
cmd.assert()
.failure()
+ .code(1)
.stderr(predicate::str::contains("No such file or directory"));
Ok(())