DelSkayn / rquickjs

High level bindings to the quickjs javascript engine
MIT License
434 stars 59 forks source link

0.2 -> 0.3 API regression: can't take `self` by value #197

Closed RReverser closed 11 months ago

RReverser commented 11 months ago

Prior to 0.3, I could take structs by value where it made sense by using quickjs(cloneable):

use rquickjs::bind;

#[bind(object)]
mod bindings {
    #[derive(Clone, Copy)]
    #[quickjs(cloneable)]
    pub struct MyStruct {
        value: u32,
    }

    impl MyStruct {
        pub fn into_inner(self) -> u32 {
            self.value
        }
    }
}

With 0.3.0, this no longer compiles regardless of whether I have cloneable or not:

error[E0277]: the trait bound `SelfMethod<MyStruct, fn(MyStruct) -> u32 {MyStruct::into_inner}>: AsFunction<'_, _, _>` is not satisfied
   --> src/lib.rs:3:1
    |
3   | #[bind(object)]
    | ^^^^^^^^^^^^^^^ the trait `AsFunction<'_, _, _>` is not implemented for `SelfMethod<MyStruct, fn(MyStruct) -> u32 {MyStruct::into_inner}>`
    |
    = help: the following other types implement trait `AsFunction<'js, A, R>`:
              <SelfMethod<T, F> as AsFunction<'js, (T, A), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T, A, B), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T, A, B, D), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T, A, B, D, E), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T, A, B, D, E, G), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T, A, B, D, E, G, H), R>>
              <SelfMethod<T, F> as AsFunction<'js, (T,), R>>
    = note: required for `Func<(&str, SelfMethod<MyStruct, fn(MyStruct) -> u32 {MyStruct::into_inner}>, PhantomData<(_, _)>)>` to implement `rquickjs::IntoJs<'_>`
note: required by a bound in `rquickjs::Object::<'js>::set`
   --> /home/rreverser/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rquickjs-core-0.3.1/src/value/object.rs:121:37
    |
121 |     pub fn set<K: IntoAtom<'js>, V: IntoJs<'js>>(&self, key: K, value: V) -> Result<()> {
    |                                     ^^^^^^^^^^^ required by this bound in `Object::<'js>::set`
    = note: this error originates in the attribute macro `bind` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `temp2` (lib) due to previous error
DelSkayn commented 11 months ago

This was an unavoidable regression in 0.3. This release fixed a soundness problem which previously allowed us to retrieve classes by reference, however this reference could outlive the lifetime of the class, and was therefore unsound.

The bind attribute didn't do any special code generation for different types of self arguments. This, together with the SelfMethod object I added to make self references somewhat working again, resulted in it no longer being possible to take self by value. Fixing that would have required a pretty big rewrite of the macros, so I opted to just fix the soundness issue and rewrite the macros in the next version.

0.4 once again allows taking self by value, reference and mutable reference with the new methods attribute macro.

RReverser commented 11 months ago

Ah ok. Can't use 0.4 yet due to unstable status + lack of docs for new macros, but good to hear it will be fixed again - closing this issue then.