From 5e39a7b3d9ba88c74dd22b82cc3f3b9732b99de2 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 22 Mar 2019 17:11:30 -0700 Subject: [PATCH 1/3] rewrite extending imports --- lib/runtime-core/src/import.rs | 117 +++++++++++++++++++++++++++++++ lib/runtime-core/src/instance.rs | 8 +++ 2 files changed, 125 insertions(+) diff --git a/lib/runtime-core/src/import.rs b/lib/runtime-core/src/import.rs index 59d9da555..f764dc7b7 100644 --- a/lib/runtime-core/src/import.rs +++ b/lib/runtime-core/src/import.rs @@ -1,5 +1,6 @@ use crate::export::Export; use hashbrown::{hash_map::Entry, HashMap}; +use std::collections::VecDeque; use std::{ cell::{Ref, RefCell}, rc::Rc, @@ -7,6 +8,8 @@ use std::{ pub trait LikeNamespace { fn get_export(&self, name: &str) -> Option; + fn get_exports(&self) -> Vec<(String, Export)>; + fn maybe_insert(&mut self, name: &str, export: Export) -> Option<()>; } pub trait IsExport { @@ -97,6 +100,53 @@ impl ImportObject { map: Rc::clone(&self.map), } } + + fn get_objects(&self) -> VecDeque<(String, String, Export)> { + let mut out = VecDeque::new(); + for (name, ns) in self.map.borrow().iter() { + for (id, exp) in ns.get_exports() { + out.push_back((name.clone(), id, exp)); + } + } + out + } +} + +pub struct ImportObjectIterator { + elements: VecDeque<(String, String, Export)>, +} + +impl Iterator for ImportObjectIterator { + type Item = (String, String, Export); + fn next(&mut self) -> Option { + self.elements.pop_front() + } +} + +impl IntoIterator for ImportObject { + type IntoIter = ImportObjectIterator; + type Item = (String, String, Export); + + fn into_iter(self) -> Self::IntoIter { + ImportObjectIterator { + elements: self.get_objects(), + } + } +} + +impl Extend<(String, String, Export)> for ImportObject { + fn extend>(&mut self, iter: T) { + let mut map = self.map.borrow_mut(); + for (ns, id, exp) in iter.into_iter() { + if let Some(like_ns) = map.get_mut(&ns) { + like_ns.maybe_insert(&id, exp).expect(&format!("Insert failed. Duplicate name {} found in namespace {}", id, ns)); + } else { + let mut new_ns = Namespace::new(); + new_ns.insert(id, exp); + map.insert(ns, Box::new(new_ns)); + } + } + } } pub struct Namespace { @@ -123,4 +173,71 @@ impl LikeNamespace for Namespace { fn get_export(&self, name: &str) -> Option { self.map.get(name).map(|is_export| is_export.to_export()) } + + fn get_exports(&self) -> Vec<(String, Export)> { + self.map + .iter() + .map(|(k, v)| (k.clone(), v.to_export())) + .collect() + } + + fn maybe_insert(&mut self, name: &str, export: Export) -> Option<()> { + if self.map.contains_key(name) { + None + } else { + self.map.insert(name.to_owned(), Box::new(export)); + Some(()) + } + } +} + +#[cfg(test)] +mod test { + use crate::global::Global; + use crate::types::Value; + + #[test] + fn extending_works() { + let mut imports1 = imports! { + "dog" => { + "happy" => Global::new(Value::I32(0)), + }, + }; + + let imports2 = imports! { + "dog" => { + "small" => Global::new(Value::I32(2)), + }, + "cat" => { + "small" => Global::new(Value::I32(3)), + }, + }; + + imports1.extend(imports2); + + let cat_ns = imports1.get_namespace("cat").unwrap(); + assert!(cat_ns.get_export("small").is_some()); + + let dog_ns = imports1.get_namespace("dog").unwrap(); + assert!(dog_ns.get_export("happy").is_some()); + assert!(dog_ns.get_export("small").is_some()); + } + + #[test] + #[should_panic] + fn extending_conflict_panics() { + let mut imports1 = imports! { + "dog" => { + "happy" => Global::new(Value::I32(0)), + }, + }; + + let imports2 = imports! { + "dog" => { + "happy" => Global::new(Value::I32(4)), + }, + }; + + imports1.extend(imports2); + } } diff --git a/lib/runtime-core/src/instance.rs b/lib/runtime-core/src/instance.rs index 3dee6ee50..ba1dcba37 100644 --- a/lib/runtime-core/src/instance.rs +++ b/lib/runtime-core/src/instance.rs @@ -427,6 +427,14 @@ impl LikeNamespace for Instance { Some(self.inner.get_export_from_index(&self.module, export_index)) } + + fn get_exports(&self) -> Vec<(String, Export)> { + unimplemented!("Use the exports method instead"); + } + + fn maybe_insert(&mut self, _name: &str, _export: Export) -> Option<()> { + None + } } /// A representation of an exported WebAssembly function. From d037c5fdbbe954459d40892aedd8245c1d666c6e Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 22 Mar 2019 17:29:09 -0700 Subject: [PATCH 2/3] fmt --- lib/runtime-core/src/import.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/runtime-core/src/import.rs b/lib/runtime-core/src/import.rs index f764dc7b7..a8dbe72b4 100644 --- a/lib/runtime-core/src/import.rs +++ b/lib/runtime-core/src/import.rs @@ -139,7 +139,10 @@ impl Extend<(String, String, Export)> for ImportObject { let mut map = self.map.borrow_mut(); for (ns, id, exp) in iter.into_iter() { if let Some(like_ns) = map.get_mut(&ns) { - like_ns.maybe_insert(&id, exp).expect(&format!("Insert failed. Duplicate name {} found in namespace {}", id, ns)); + like_ns.maybe_insert(&id, exp).expect(&format!( + "Insert failed. Duplicate name {} found in namespace {}", + id, ns + )); } else { let mut new_ns = Namespace::new(); new_ns.insert(id, exp); From 225b82ae785821eb9b8d18fd327d549b6047ac06 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Sun, 24 Mar 2019 17:16:05 -0700 Subject: [PATCH 3/3] change behavior of extend to overwrite on conflict --- lib/runtime-core/src/import.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/runtime-core/src/import.rs b/lib/runtime-core/src/import.rs index a8dbe72b4..4c8401f06 100644 --- a/lib/runtime-core/src/import.rs +++ b/lib/runtime-core/src/import.rs @@ -139,10 +139,7 @@ impl Extend<(String, String, Export)> for ImportObject { let mut map = self.map.borrow_mut(); for (ns, id, exp) in iter.into_iter() { if let Some(like_ns) = map.get_mut(&ns) { - like_ns.maybe_insert(&id, exp).expect(&format!( - "Insert failed. Duplicate name {} found in namespace {}", - id, ns - )); + like_ns.maybe_insert(&id, exp); } else { let mut new_ns = Namespace::new(); new_ns.insert(id, exp); @@ -185,17 +182,14 @@ impl LikeNamespace for Namespace { } fn maybe_insert(&mut self, name: &str, export: Export) -> Option<()> { - if self.map.contains_key(name) { - None - } else { - self.map.insert(name.to_owned(), Box::new(export)); - Some(()) - } + self.map.insert(name.to_owned(), Box::new(export)); + Some(()) } } #[cfg(test)] mod test { + use crate::export::Export; use crate::global::Global; use crate::types::Value; @@ -227,8 +221,7 @@ mod test { } #[test] - #[should_panic] - fn extending_conflict_panics() { + fn extending_conflict_overwrites() { let mut imports1 = imports! { "dog" => { "happy" => Global::new(Value::I32(0)), @@ -242,5 +235,14 @@ mod test { }; imports1.extend(imports2); + let dog_ns = imports1.get_namespace("dog").unwrap(); + + assert!( + if let Export::Global(happy_dog_global) = dog_ns.get_export("happy").unwrap() { + happy_dog_global.get() == Value::I32(4) + } else { + false + } + ); } }