Merge #851

851: fix: Handle errors when pushing a BTreeMap r=Marwes a=Marwes

Also move the insert_string hack into std.map so std.json.de does not
need to be imported

Fixes #847

Co-authored-by: Markus Westerlind <[email protected]>

Author: bors[bot]

std/json/de.glu

@@ -253,9 +253,6 @@ let run ?de value : [Deserialize a] -> Value -> Result Error a =
     do state = de.deserializer value
     Ok state.value
 
-#[doc(hidden)]
-let insert_string : String -> a -> Map String a -> Map String a = std_map.insert
-
 let bool_deserializer : Deserialize Bool = { deserializer = bool }
 
 let int_deserializer : Deserialize Int = { deserializer = int }
@@ -317,6 +314,4 @@ let map_deserializer : [Deserialize a] -> Deserialize (Map String a) =
     list_deserializer,
     array_deserializer,
     map_deserializer,
-
-    insert_string,
 }

std/map.glu

@@ -124,6 +124,11 @@ let keys : [Ord k] -> Map k a -> List k = foldr_with_key (\k _ acc -> Cons k acc
 /// Returns a list of all values in the map.
 let values : [Ord k] -> Map k a -> List a = foldr Cons Nil
 
+
+
+#[doc(hidden)]
+let insert_string : String -> a -> Map String a -> Map String a = insert
+
 {
     Map,
 
@@ -146,4 +151,6 @@ let values : [Ord k] -> Map k a -> List a = foldr Cons Nil
     to_list,
     keys,
     values,
+
+    insert_string,
 }

tests/api.rs

@@ -6,13 +6,13 @@ extern crate gluon_vm;
 #[macro_use]
 extern crate gluon_codegen;
 
-use std::sync::Arc;
+use std::{collections::BTreeMap, sync::Arc};
 
 use futures::prelude::*;
 
 use gluon::{
     base::types::{Alias, ArcType, Type},
-    import::{add_extern_module, Import},
+    import::{add_extern_module, add_extern_module_with_deps, Import},
     query::Compilation,
     vm::{
         api::{
@@ -368,6 +368,41 @@ fn use_rust_created_record_as_polymorphic() {
     assert_eq!(result, 1);
 }
 
+#[test]
+fn return_btreemap() {
+    let _ = ::env_logger::try_init();
+
+    let vm = make_vm();
+
+    add_extern_module_with_deps(
+        &vm,
+        "test",
+        |vm| {
+            ExternModule::new(
+                vm,
+                primitive!(1, "test", |()| {
+                    vec![("a".to_string(), 1), ("b".to_string(), 2)]
+                        .into_iter()
+                        .collect::<BTreeMap<_, _>>()
+                }),
+            )
+        },
+        vec!["std.map".into(), "std.json.de".into()],
+    );
+
+    vm.run_expr::<()>("", "let _ = import! test in ()")
+        .unwrap_or_else(|err| panic!("{}", err));
+    let (result, _) = vm
+        .run_expr::<BTreeMap<_, _>>("", "(import! test) ()")
+        .unwrap_or_else(|err| panic!("{}", err));
+    assert_eq!(
+        result,
+        vec![("a".to_string(), 1), ("b".to_string(), 2)]
+            .into_iter()
+            .collect::<BTreeMap<_, _>>()
+    );
+}
+
 #[test]
 fn get_value_boxed_or_unboxed() {
     let _ = ::env_logger::try_init();

vm/src/api/mod.rs

@@ -512,8 +512,9 @@ where
         lock: Lock,
         _: VmIndex,
     ) -> Poll<Result<()>> {
+        let result = self.push(context);
         context.stack().release_lock(lock);
-        Poll::Ready(self.push(context))
+        Poll::Ready(result)
     }
 }
 
@@ -1316,13 +1317,14 @@ where
     type Map<V2> = OpaqueValue<RootedThread, BTreeMap<String, V2>>;
     let mut map: Map<V> = thread.get_global("std.map.empty")?;
     let mut insert: OwnedFunction<fn(String, V, Map<V>) -> Map<V>> =
-        thread.get_global("std.json.de.insert_string")?;
+        thread.get_global("std.map.insert_string")?;
 
-    context.drop();
-    for (key, value) in map_iter {
-        map = insert.call(key.borrow().to_string(), value, map)?;
-    }
-    context.restore();
+    map = context.release_for(|| {
+        for (key, value) in map_iter {
+            map = insert.call(key.borrow().to_string(), value, map)?;
+        }
+        Ok::<_, Error>(map)
+    })?;
 
     map.push(context)
 }

vm/src/thread.rs

@@ -2333,10 +2333,11 @@ impl<'b, 'gc> ExecuteContext<'b, 'gc> {
                     let data_tag = match self.stack.top().get_repr() {
                         Data(data) => data.tag(),
                         ValueRepr::Tag(tag) => *tag,
-                        _ => {
-                            return Err(Error::Message(
-                                "Op TestTag called on non data type".to_string(),
-                            ))
+                        data => {
+                            return Err(Error::Message(format!(
+                                "Op TestTag called on non data type: {:?}",
+                                data
+                            )))
                             .into();
                         }
                     };
@@ -2859,12 +2860,11 @@ pub struct ActiveThread<'vm> {
 }
 
 impl<'vm> ActiveThread<'vm> {
-    pub fn drop(&mut self) {
+    pub fn release_for<R>(&mut self, f: impl FnOnce() -> R) -> R {
         self.context = None;
-    }
-
-    pub fn restore(&mut self) {
+        let r = f();
         *self = self.thread.current_context();
+        r
     }
 
     pub fn thread(&self) -> &'vm Thread {