ext-php-rs icon indicating copy to clipboard operation
ext-php-rs copied to clipboard

Lifetime issue when trying to reference both "this" and another "php_impl"

Open veewee opened this issue 2 years ago • 2 comments

Hello there,

Absolute Rust noob out here! ;)

I'm playing around with these bindings and notice I can't really get a way around this structure:

#[php_class(name="Wasm\\InstanceBuilder")]
#[derive(Default)]
pub struct InstanceBuilder {
    pub wat: Box<String>,
    pub imports : Option<Box<WasmImports>>,
}

#[php_impl]
impl InstanceBuilder {    
    pub fn from_wat(wat: String) -> InstanceBuilder {
        InstanceBuilder {
            wat: wat.clone().into(),
            ..Default::default()
        }.into()
    }

    pub fn with_imports(
        #[this] this: &mut ZendClassObject<InstanceBuilder>,
        imports: &mut ZendClassObject<WasmImports>
    ) -> &mut ZendClassObject<InstanceBuilder> {
        this.imports = Some((*imports).clone().into());

        this
    }
}

which should allow thsese PHP lines:

$builder = Wasm\InstanceBuilder::fromWat('(module)')->withImports(
  Wasm\Imports::create()
);

The problem here is lifetimes:

error[E0106]: missing lifetime specifier
  --> src/lib.rs:86:10
   |
84 |         #[this] this: &mut ZendClassObject<InstanceBuilder>,
   |                       -------------------------------------
85 |         imports: &mut ZendClassObject<WasmImports>
   |                  ---------------------------------
86 |     ) -> &mut ZendClassObject<InstanceBuilder> {
   |          ^ expected named lifetime parameter
   |
   = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `this` or `imports`
help: consider introducing a named lifetime parameter
   |
83 ~     pub fn with_imports<'a>(
84 ~         #[this] this: &'a mut ZendClassObject<InstanceBuilder>,
85 ~         imports: &'a mut ZendClassObject<WasmImports>
86 ~     ) -> &'a mut ZendClassObject<InstanceBuilder> {

But lifetimes are not supported according to the docs. Yet stubburon as I am, I try to do it anyways, resulting in:

74 | #[php_impl]
   | ^^^^^^^^^^^-
   | |          |
   | |          lifetime `'a` is missing in item created through this procedural macro
   | |          help: consider introducing lifetime `'a` here: `<'a>`
   | undeclared lifetime
   |
   = note: this error originates in the attribute macro `php_impl` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0261]: use of undeclared lifetime name `'a`
   --> src/lib.rs:210:1
    |
210 | #[php_module]
    | ^^^^^^^^^^^^^- lifetime `'a` is missing in item created through this procedural macro
    | |
    | undeclared lifetime
    |
    = note: this error originates in the attribute macro `php_module` (in Nightly builds, run with -Z macro-backtrace for more info)

So I am getting a bit stuck on this concept and think its more of a limitation at the moment of these bindings. The docs suggest using an Arc for this, but wrapping an Arc around it - yet this happens on method binding, and there is no From for imports: Arc<&mut ZendClassObject<WasmImports>>

^^^^^^^^^^^^^ the trait `FromZval<'_>` is not implemented for `Arc<&mut ZendClassObject<WasmImports>>`

So my questions here are:

  • What is the suggested way of having this kind of multiple mutable references in these PHP methods?
  • Are there any examples on this?
  • Is there any way around this or should it be tackled in this package instead?

Thanks for this package, it's very cool!

FYI - a bit more context about the above code can be found here: https://github.com/veewee/ext-wasm/pull/23

veewee avatar Apr 22 '23 09:04 veewee

Hello can you try to return a reference of your builder in from_wat? If it works, I'll show you why

ptondereau avatar Apr 22 '23 10:04 ptondereau

Unless I misunderstood, That doesn't seem to work. Tried returning &InstanceBuilder, &mut InstanceBuilder, &'static mut InstanceBuilder. None of them implement IntoZval

For example:

trait `IntoZval` is not implemented for `&InstanceBuilder`

InstanceBuilder itself does implement ZvalConvert, but not the referenced version.

veewee avatar Apr 22 '23 11:04 veewee