Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about proper way to marshal multi-parameter type constructors #719

Closed
tyler-conrad opened this issue Apr 25, 2019 · 3 comments · Fixed by #720 or #754
Closed

Question about proper way to marshal multi-parameter type constructors #719

tyler-conrad opened this issue Apr 25, 2019 · 3 comments · Fixed by #720 or #754
Labels

Comments

@tyler-conrad
Copy link

I've been trying to marshall a type constructor with 2 parameters and it appears that the stack is not aligning correctly. I'm probably not using the Getable and Pushable traits correctly, any pointers would be great. Here is some example code outlining the issue:

#[macro_use]
extern crate gluon_codegen;

#[macro_use]
extern crate gluon_vm;

use gluon::{
    base::types::ArcType,
    import::add_extern_module,
    new_vm,
    vm::{
        api::{ActiveThread, Getable, Pushable, UserdataValue, ValueRef, VmType},
        internal::Value,
        thread::Thread,
        ExternModule, Result, Variants,
    },
    Compiler,
};

#[derive(Debug, Clone)]
struct ExternalValue(f64);

#[derive(Debug, Clone, Userdata)]
struct ValueWrapper(ExternalValue);

impl ValueWrapper {
    fn new(_: ()) -> ValueWrapper {
        ValueWrapper(ExternalValue(0.0))
    }

    fn load(vm: &Thread) -> Result<ExternModule> {
        vm.register_type::<ValueWrapper>("Value", &[])?;

        ExternModule::new(
            vm,
            record! {
                type Value => ValueWrapper,
                new => primitive!(1, ValueWrapper::new),
            },
        )
    }
}

#[derive(Debug)]
enum ExternalEnumOne {
    One,
    Two(f64, f64),
}

#[derive(Debug)]
struct EnumOneWrapper(ExternalEnumOne);

impl VmType for EnumOneWrapper {
    type Type = Self;

    fn make_type(vm: &Thread) -> ArcType {
        vm.find_type_info("example.types.EnumOne")
            .expect("Could not find type EnumOneWrapper")
            .into_type()
    }
}

impl<'vm> Pushable<'vm> for EnumOneWrapper {
    fn push(self, context: &mut ActiveThread<'vm>) -> Result<()> {
        match self.0 {
            ExternalEnumOne::One => context.push(Value::tag(0)),
            ExternalEnumOne::Two(one, two) => {
                one.push(context)?;
                two.push(context)?;
                let thread = context.thread();
                context.context().push_new_data(thread, 1, 2).unwrap();
            }
        };
        Ok(())
    }
}

impl<'vm, 'value> Getable<'vm, 'value> for EnumOneWrapper {
    fn from_value(_vm: &'vm Thread, data: Variants<'value>) -> EnumOneWrapper {
        match data.as_ref() {
            ValueRef::Data(data) => match data.tag() {
                0 => EnumOneWrapper(ExternalEnumOne::One),
                1 => EnumOneWrapper(ExternalEnumOne::Two(
                    match data.get(0).unwrap() {
                        ValueRef::Float(value) => value,
                        _ => panic!("Failed to marshall ExternalEnumOne::Two"),
                    },
                    match data.get(1).unwrap() {
                        ValueRef::Float(value) => value,
                        _ => panic!("Failed to marshall ExternalEnumOne::Two"),
                    },
                )),
                _ => panic!("Invalid VmTag for EnumOneWrapper."),
            },
            _ => panic!("EnumOneWrapper is not a complex type."),
        }
    }
}

#[derive(Debug)]
enum ExternalEnumTwo {
    A(ExternalEnumOne),
    B {
        value: ExternalValue,
        enum_one: ExternalEnumOne,
    },
}

#[derive(Debug)]
struct EnumTwoWrapper(ExternalEnumTwo);

impl EnumTwoWrapper {
    fn id(self) -> EnumTwoWrapper {
        self
    }

    fn load(vm: &Thread) -> Result<ExternModule> {
        ExternModule::new(
            vm,
            record! {
                id => primitive!(1, EnumTwoWrapper::id),
            },
        )
    }
}

impl VmType for EnumTwoWrapper {
    type Type = Self;

    fn make_type(vm: &Thread) -> ArcType {
        vm.find_type_info("example.types.EnumTwo")
            .expect("Could not find type EnumTwoWrapper")
            .into_type()
    }
}

impl<'vm> Pushable<'vm> for EnumTwoWrapper {
    fn push(self, context: &mut ActiveThread<'vm>) -> Result<()> {
        match self.0 {
            ExternalEnumTwo::A(value) => {
                EnumOneWrapper(value).push(context)?;
                let thread = context.thread();
                context.context().push_new_data(thread, 0, 1).unwrap();
            }
            ExternalEnumTwo::B { value, enum_one } => {
                (record! {
                    value => ValueWrapper(value),
                    enum_one => EnumOneWrapper(enum_one)
                })
                .push(context)?;
                let thread = context.thread();
                context.context().push_new_data(thread, 1, 1).unwrap();
            }
        };
        Ok(())
    }
}

impl<'vm, 'value> Getable<'vm, 'value> for EnumTwoWrapper {
    fn from_value(vm: &'vm Thread, data: Variants<'value>) -> EnumTwoWrapper {
        match data.as_ref() {
            ValueRef::Data(data) => match data.tag() {
                0 => EnumTwoWrapper(ExternalEnumTwo::A(
                    EnumOneWrapper::from_value(vm, data.get_variant(0).unwrap()).0,
                )),
                1 => match data.get(0).unwrap() {
                    ValueRef::Data(data) => {
                        let UserdataValue(ValueWrapper(external_value)) =
                            <UserdataValue<ValueWrapper>>::from_value(
                                vm,
                                data.lookup_field(vm, "value").unwrap(),
                            );
                        EnumTwoWrapper(ExternalEnumTwo::B {
                            value: external_value,
                            enum_one: EnumOneWrapper::from_value(
                                vm,
                                data.lookup_field(vm, "enum_one").unwrap(),
                            )
                            .0,
                        })
                    }
                    _ => panic!("Unable to marshall ExternalEnumTwo::B."),
                },
                _ => panic!("Invalid VmTag for EnumTwoWrapper."),
            },
            _ => panic!("EnumTwoWrapper is not a complex type."),
        }
    }
}

fn main() {
    let vm = new_vm();
    let mut compiler = Compiler::new();

    add_extern_module(&vm, "example.value", ValueWrapper::load);

    let src = r#"
let { Value } = import! example.value

type EnumOne = | One | Two Float Float
type EnumTwo = | A EnumOne | B { value: Value, enum_one: EnumOne }

{
    EnumOne,
    EnumTwo,
}
"#;
    compiler.load_script(&vm, "example.types", src).unwrap();
    add_extern_module(&vm, "example.enum_two.prim", EnumTwoWrapper::load);

    let script1 = r#"
let { EnumOne, EnumTwo } = import! example.types
let { id } = import! example.enum_two.prim
id (A One)
"#;
    let (value, _) = compiler
        .run_expr::<EnumTwoWrapper>(&vm, "example1", script1)
        .unwrap();
    println!("{:#?}", value);

    let script2 = r#"
let { new } = import! example.value
let { EnumOne, EnumTwo } = import! example.types
let { id } = import! example.enum_two.prim
id (B { value = new (), enum_one = One })
"#;
    let (value, _) = compiler
        .run_expr::<EnumTwoWrapper>(&vm, "example2", script2)
        .unwrap();
    println!("{:#?}", value);

    let script3 = r#"
let { new } = import! example.value
let { EnumOne, EnumTwo } = import! example.types
let { id } = import! example.enum_two.prim
id (B { value = new (), enum_one = Two 0.0 0.0 })
"#;
    let (value, _) = compiler
        .run_expr::<EnumTwoWrapper>(&vm, "example3", script3)
        .unwrap();
    println!("{:#?}", value);
}

I get the error 'ValueRef is not an Userdata' and it looks like one of the arguments passed to the 'Two' constructor is causing a misalignment of the stack. I modified gluon locally to print out the data that is trying to get interpreted as Userdata and it shows Float(0.0). Here is the output of running the script:

EnumTwoWrapper(
    A(
        One
    )
)
EnumTwoWrapper(
    B {
        value: ExternalValue(
            0.0
        ),
        enum_one: One
    }
)
thread 'main' panicked at 'ValueRef is not an Userdata: Float(
    0.0
)', /home/tyler/Desktop/vulkan/gluon/vm/src/api/mod.rs:629:22
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:59
             at src/libstd/panicking.rs:211
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:491
   5: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:398
   6: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:353
   7: <&'value T as gluon_vm::api::Getable<'vm, 'value>>::from_value
             at ./<::std::macros::panic macros>:8
   8: <gluon_vm::api::UserdataValue<T> as gluon_vm::api::Getable<'vm, 'value>>::from_value
             at /home/tyler/Desktop/vulkan/gluon/vm/src/api/mod.rs:601
   9: <marshall_test::EnumTwoWrapper as gluon_vm::api::Getable<'vm, 'value>>::from_value
             at src/main.rs:169
  10: gluon::Compiler::run_expr::{{closure}}
             at /home/tyler/Desktop/vulkan/gluon/src/lib.rs:602
  11: <futures::future::and_then::AndThen<A, B, F> as futures::future::Future>::poll::{{closure}}::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/future/and_then.rs:34
  12: <core::result::Result<T, E>>::map
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libcore/result.rs:468
  13: <futures::future::and_then::AndThen<A, B, F> as futures::future::Future>::poll::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/future/and_then.rs:33
  14: <futures::future::chain::Chain<A, B, C>>::poll
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/future/chain.rs:39
  15: <futures::future::and_then::AndThen<A, B, F> as futures::future::Future>::poll
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/future/and_then.rs:32
  16: <futures::task_impl::Spawn<T>>::poll_future_notify::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/mod.rs:329
  17: <futures::task_impl::Spawn<T>>::enter::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/mod.rs:399
  18: futures::task_impl::std::set
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/std/mod.rs:78
  19: <futures::task_impl::Spawn<T>>::enter
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/mod.rs:399
  20: <futures::task_impl::Spawn<T>>::poll_fn_notify
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/mod.rs:291
  21: <futures::task_impl::Spawn<T>>::poll_future_notify
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/mod.rs:329
  22: futures::task_impl::std::<impl futures::task_impl::Spawn<F>>::wait_future::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/std/mod.rs:231
  23: futures::task_impl::std::ThreadNotify::with_current::{{closure}}
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/std/mod.rs:478
  24: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/thread/local.rs:309
  25: <std::thread::local::LocalKey<T>>::with
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/thread/local.rs:255
  26: futures::task_impl::std::ThreadNotify::with_current
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/std/mod.rs:478
  27: futures::task_impl::std::<impl futures::task_impl::Spawn<F>>::wait_future
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/task_impl/std/mod.rs:228
  28: futures::future::Future::wait
             at /home/tyler/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/futures-0.1.26/src/future/mod.rs:299
  29: gluon::Compiler::run_expr
             at /home/tyler/Desktop/vulkan/gluon/src/lib.rs:598
  30: marshall_test::main
             at src/main.rs:238
  31: std::rt::lang_start::{{closure}}
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/rt.rs:74
  32: std::panicking::try::do_call
             at src/libstd/rt.rs:59
             at src/libstd/panicking.rs:310
  33: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:102
  34: std::rt::lang_start_internal
             at src/libstd/panicking.rs:289
             at src/libstd/panic.rs:398
             at src/libstd/rt.rs:58
  35: std::rt::lang_start
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/rt.rs:74
  36: main
  37: __libc_start_main
  38: _start
Marwes added a commit to Marwes/gluon that referenced this issue Apr 27, 2019
The generated code accounted for this but by moving it in we get a
simpler interface.

Fixes gluon-lang#719
@Marwes
Copy link
Member

Marwes commented Apr 27, 2019

Fixed with #720. Best I can tell these implementations could be automatically derived though? Why did you feel the need to implement them manually?

bors bot added a commit that referenced this issue Apr 27, 2019
720: fix(vm): Automatically remove the elements added to pushed data r=Marwes a=Marwes

The generated code accounted for this but by moving it in we get a
simpler interface.

Fixes #719

Co-authored-by: Markus Westerlind <marwes91@gmail.com>
@bors bors bot closed this as completed in #720 Apr 27, 2019
@tyler-conrad
Copy link
Author

Thanks for the quick response. I'm working on writing a wrapper for the winit library and that's where I came across this problem. Since I'm wrapping types that are from an external crate my understanding is that I cannot derive Getable or Pushable due to the fact that all of the fields of these types also need to be Getable or Pushable but I can't implement that trait on the external types due to orphan rules.

@Marwes
Copy link
Member

Marwes commented May 1, 2019

Yeah, that is a big problem I want to tackle at some point by adding attributes to the derive macros like serde's with attributes.

Would also like to be able to point a build script at a plain rust file and have bindings be generated for all (or a subset) of the public functions in it

Marwes added a commit to Marwes/gluon that referenced this issue Jul 6, 2019
<a name="v0.12.0"></a>
## v0.12.0 (2019-07-06)

#### Bug Fixes

*   Remove Userdata and Trace impls for RwLock and Mutex ([e90f02b](gluon-lang@e90f02b))
*   Add missing negate function from the prelude ([0091f47](gluon-lang@0091f47))
*   Refer to registered types by their full name ([a2daace](gluon-lang@a2daace), breaks [#](https://github.com/gluon-lang/gluon/issues/))
*   Handle newtypes with a public field ([d1fef96](gluon-lang@d1fef96), closes [gluon-lang#713](gluon-lang#713))
*   Don't ICE on unapplied, aliased constructors ([2a44a0d](gluon-lang@2a44a0d))
* **check:**
  *  Propagate metadata through parens ([bd767c0](gluon-lang@bd767c0))
  *  Bring nested implicit instances into scope ([ad82bde](gluon-lang@ad82bde))
  *  Don't lose type information in catch-all ([d2a3fbf](gluon-lang@d2a3fbf), closes [gluon-lang#702](gluon-lang#702), [gluon-lang#703](gluon-lang#703), [gluon-lang#704](gluon-lang#704), [gluon-lang#705](gluon-lang#705))
* **codegen:**  Return exactly the same type on VmType derive on enum ([375d3e9](gluon-lang@375d3e9))
* **compiler:**  Don't panic when matching a tuple against an alias ([777bd31](gluon-lang@777bd31), closes [gluon-lang#749](gluon-lang#749))
* **std:**
  *  cleaned up statet.glu exports ([5d8864f](gluon-lang@5d8864f))
  *  export wrap_monad from transformer.glu ([0e9d7bc](gluon-lang@0e9d7bc))
* **vm:**
  *  Check if a collection is needed when creating a child thread ([86e4b9f](gluon-lang@86e4b9f))
  *  Automatically remove the elements added to pushed data ([8cd5152](gluon-lang@8cd5152), closes [gluon-lang#719](gluon-lang#719))

#### Performance

*   Use NonNull for garbage collected pointers ([9c66ede](gluon-lang@9c66ede))
*   Don't recurse into already visited records to find implicits ([b50061f](gluon-lang@b50061f))
*   Avoid recursing into non-implicit types ([c35b29e](gluon-lang@c35b29e))
*   Use SmallVec in Partition ([d8c549b](gluon-lang@d8c549b))
*   Use a scoped collections over a persistent in implicit resolution ([d13097e](gluon-lang@d13097e))
*   Memoize implicit attribute lookups (-3%) ([254af75](gluon-lang@254af75))
*   Speedup Symbol::module ([9566a37](gluon-lang@9566a37))
*   Avoid creating function types unnecessarily ([170f467](gluon-lang@170f467))
* **compiler:**
  *  Shrink the core::Expr type to 40 bytes (from 72) ([779d1b6](gluon-lang@779d1b6))
  *  Copy elements directly into arena ([cd2dd36](gluon-lang@cd2dd36))

#### Features

*   Add gc::Mutex ([d6e1246](gluon-lang@d6e1246))
*   Automatically unroot values stored in `Gc` allocated values ([6ebc398](gluon-lang@6ebc398), closes [gluon-lang#746](gluon-lang#746))
*   Add derive for Traverseable ([844418d](gluon-lang@844418d))
*   Allow mutable references to be passed to gluon ([602220b](gluon-lang@602220b))
*   Add std.env ([b561c8d](gluon-lang@b561c8d))
*   Implement woobly type propagation ([a0b8452](gluon-lang@a0b8452))
* **codegen:**  Add the newtype attribute ([4085463](gluon-lang@4085463))
* **completion:**
  *  Match on the symbols in type declarations ([9d28ba1](gluon-lang@9d28ba1))
  *  Return scoped symbols in the all_symbols query ([94a385a](gluon-lang@94a385a))
  *  Match on the symbols in type declarations ([8fe083a](gluon-lang@8fe083a))
  *  Return scoped symbols in the all_symbols query ([1ad302b](gluon-lang@1ad302b))
* **doc:**  Link to the github source ([da75875](gluon-lang@da75875))
* **vm:**  Allow references to be passed through ([3a92b17](gluon-lang@3a92b17))

#### Breaking Changes

*   Refer to registered types by their full name ([a2daace](gluon-lang@a2daace), breaks [#](https://github.com/gluon-lang/gluon/issues/))
@Marwes Marwes mentioned this issue Jul 6, 2019
bors bot added a commit that referenced this issue Jul 6, 2019
754: Version 0.12.0 r=Marwes a=Marwes

<a name="v0.12.0"></a>
## v0.12.0 (2019-07-06)

#### Bug Fixes

*   Remove Userdata and Trace impls for RwLock and Mutex ([e90f02b](e90f02b))
*   Add missing negate function from the prelude ([0091f47](0091f47))
*   Refer to registered types by their full name ([a2daace](a2daace), breaks [#](https://github.com/gluon-lang/gluon/issues/))
*   Handle newtypes with a public field ([d1fef96](d1fef96), closes [#713](#713))
*   Don't ICE on unapplied, aliased constructors ([2a44a0d](2a44a0d))
* **check:**
  *  Propagate metadata through parens ([bd767c0](bd767c0))
  *  Bring nested implicit instances into scope ([ad82bde](ad82bde))
  *  Don't lose type information in catch-all ([d2a3fbf](d2a3fbf), closes [#702](#702), [#703](#703), [#704](#704), [#705](#705))
* **codegen:**  Return exactly the same type on VmType derive on enum ([375d3e9](375d3e9))
* **compiler:**  Don't panic when matching a tuple against an alias ([777bd31](777bd31), closes [#749](#749))
* **std:**
  *  cleaned up statet.glu exports ([5d8864f](5d8864f))
  *  export wrap_monad from transformer.glu ([0e9d7bc](0e9d7bc))
* **vm:**
  *  Check if a collection is needed when creating a child thread ([86e4b9f](86e4b9f))
  *  Automatically remove the elements added to pushed data ([8cd5152](8cd5152), closes [#719](#719))

#### Performance

*   Use NonNull for garbage collected pointers ([9c66ede](9c66ede))
*   Don't recurse into already visited records to find implicits ([b50061f](b50061f))
*   Avoid recursing into non-implicit types ([c35b29e](c35b29e))
*   Use SmallVec in Partition ([d8c549b](d8c549b))
*   Use a scoped collections over a persistent in implicit resolution ([d13097e](d13097e))
*   Memoize implicit attribute lookups (-3%) ([254af75](254af75))
*   Speedup Symbol::module ([9566a37](9566a37))
*   Avoid creating function types unnecessarily ([170f467](170f467))
* **compiler:**
  *  Shrink the core::Expr type to 40 bytes (from 72) ([779d1b6](779d1b6))
  *  Copy elements directly into arena ([cd2dd36](cd2dd36))

#### Features

*   Add gc::Mutex ([d6e1246](d6e1246))
*   Automatically unroot values stored in `Gc` allocated values ([6ebc398](6ebc398), closes [#746](#746))
*   Add derive for Traverseable ([844418d](844418d))
*   Allow mutable references to be passed to gluon ([602220b](602220b))
*   Add std.env ([b561c8d](b561c8d))
*   Implement woobly type propagation ([a0b8452](a0b8452))
* **codegen:**  Add the newtype attribute ([4085463](4085463))
* **completion:**
  *  Match on the symbols in type declarations ([9d28ba1](9d28ba1))
  *  Return scoped symbols in the all_symbols query ([94a385a](94a385a))
  *  Match on the symbols in type declarations ([8fe083a](8fe083a))
  *  Return scoped symbols in the all_symbols query ([1ad302b](1ad302b))
* **doc:**  Link to the github source ([da75875](da75875))
* **vm:**  Allow references to be passed through ([3a92b17](3a92b17))

#### Breaking Changes

*   Refer to registered types by their full name ([a2daace](a2daace), breaks [#](https://github.com/gluon-lang/gluon/issues/))

Co-authored-by: Markus Westerlind <marwes91@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants