How to handle mutable arguments? about safer_ffi HOT 7 CLOSED

Yes, this is the "out parameter" / "out reference" pattern, which thus involve the optional support for them that safer-ffi has:

1. Make sure the out-refs Cargo feature is enabled:

   [dependencies]
   safer-ffi.version = "…"
   safer-ffi.features = ["proc_macros", "out-refs"]

2. This will bring the Out type to ::safer_ffi::prelude::Out.

   This is a non-nullable pointer, so if it is optional, as per your:

   Ideally I'd want the reference to be nullable, so that one could call api_func(NULL) in C

   you'll have to use Option<Out<'_, …>> types.

The main API to write through an Out type is .write():

#[ffi_export]
fn api_func(error: Option<Out<'_, Error>>) {
    if let Some(out_err) = err {
        out_err.write(Error {
            error_code: 0,
            error_str: char_p::new("error"),
        });
    }
}

Remark regarding out_ref.write(…) vs. *mut_ref = …;

        out_err.write(Error {
            error_code: 0,
            error_str: char_p::new("error"),
        });

        // Error: use of moved value
        out_err.write(Error {
            error_code: 42,
            error_str: char_p::new("let's nuance it"),
        });

        /* initially: out_err: Out<'_, Error> */
        let out_err = out_err.write(Error {
            error_code: 0,
            error_str: char_p::new("error"),
        });
        /* now: out_err: &'_ mut Error */

        // OK
        *out_err = Error {
            error_code: 42,
            error_str: char_p::new("let's nuance it"),
        };

from safer_ffi.

I'm going to close the issue since I consider that the usage of the optional Out type solves your use case. But don't let the closed issue deter you from asking further questions or clarifications! 🙂 In other words, should you encounter any issue when following my indications, you are free to keep asking about it, even if the issue appears to be closed

from safer_ffi.

Thanks for the detailed response, and sorry it took me so long to come up with a feedback. This was indeed very helpful — I did not notice Out in the documentation. The only problem I'm still having is that the compiler is not smart enough to figure out that I'm not reusing the error value if .write() is called in a nested method call.

To illustrate, I have a bunch of methods that may return Option or Result, and I want to make it easy for the user to forward their outcome into the FFI error. Ideally, I would want to write something like

#[ffi_export]
fn api_func(error: Option<Out<'_, Error>>) -> Option<usize> {
    let x = returns_option().report_error("returns_option() failed", error)?;
    let y = returns_result().report_error("returns_result() failed", error)?;
    Some(x + y)
}

and report_result() is something like

impl<T, E: Display> Reportable<T> for Result<T, E> {
    fn report_error<'a>(self, message: &'a str, error: Option<Out<'a, Error>>) -> Option<T> {
        match self {
            Ok(value) => Some(value),
            Err(err) => {
                write_error(format!("{}: {}", message, err), error);
                None
            }
        }
    }
}

where write_error() consumes error. Similar implementation for Option.

The way report_error() is implemented, error is consumed on an erroneous variant of Option or Result, so theoretically the compiler should be able to deduce that the usage in api_func() is correct. And indeed it can deduce that if I inline report_error(). But without the inlining, alas, it complains about error being used after it's consumed in the first call.

I see two ways out of this, both are not completely satisfactory.

Explicit: make report_error(...) -> Option<(T, Option<Out<'a, Error>)>. So api_func() would look like

#[ffi_export]
fn api_func(error: Option<Out<'_, Error>>) -> Option<usize> {
    let (x, error) = returns_option().report_error("returns_option() failed", error)?;
    let (y, error) = returns_result().report_error("returns_result() failed", error)?;
    Some(x + y)
}

This adds visual noise to the users of report_error().

Macro: inline repor_error() every time. So api_func() would look like

#[ffi_export]
fn api_func(error: Option<Out<'_, Error>>) -> Option<usize> {
    let x = report_option!(returns_option(), "returns_option() failed", error);
    let y = report_result!(returns_result(), "returns_result() failed", error);
    Some(x + y)
}

I consider macros the absolute last resort because they're hard to maintain and debug. And it makes the code too vague.

Any ideas?

from safer_ffi.

Context

So, the way to see Out<'_, T> is as kind of like a &'_ mut T, albeit with less compiler magic and thus fewer conveniences here and there. And Option<&'_ mut T> would have a similar ownership problems here, of not being Copy, etc.
The usual way to "act as if" &mut T was kind of copyable is reborrowing, whereby you can "copy" a &'shorter mut T.

With an Option, this would be something like:

fn example<T> (mut o: Option<&mut T>)
{
    fn consumes_option<T> (_: Option<&mut T>)
    {}
    
-   consumes_option(o); /* would cause a `move` error on the next `consumes_option` */
+   consumes_option(o.as_deref_mut());
    consumes_option(o);
}

Obviously writing o.as_deref_mut() at each call remains noisy, but at that point we can thus realize we could move that operation inside the helper function:

-   fn consumes_option<T> (o: Option<&mut T>)
+   fn consumes_option_not<T> (o: &mut Option<&mut T>)

• (and use o.as_deref_mut(), the reborrowing operation, inside it)

• In practice, thanks to match ergonomics, and the magical auto-reborrowing of the inner &mut, chances are you'd never really need to explicitly call o.as_deref_mut() (maybe a .as_mut() but then inside a closure or whatever the resulting &'short mut &'long mut T would be flattened to a &'short mut T).

Well, in the case of Out<'_, T>, all of the above applies, except for the reborrowing which does not happen implicitly / magically: you can't have a &'short mut Out<'long, T> magically collapse down to a Out<'short, T>. You'll need to call .reborrow() on it explicitly.

All this thus yields:

The fix

  #[ffi_export]
  fn api_func(mut error: Option<Out<'_, Error>>) -> Option<usize> {
+             ^^^
      let x = returns_option().report_error("returns_option() failed", &mut error)?;
+                                                                      ^^^^
      let y = returns_result().report_error("returns_result() failed", &mut error)?;
+                                                                      ^^^^
      Some(x + y)
  }

• (you could even start the body with let error = &mut error; and that way you can skip the &mut s at each call site)

impl<T, E: Display> Reportable<T> for Result<T, E> {
    fn report_error<'a>(self, message: &'a str, error: &mut Option<Out<'a, Error>>) -> Option<T> {
+                                                      ^^^^
        match self {
            Ok(value) => Some(value),
            Err(err) => {
                write_error(format!("{}: {}", message, err), error.as_mut().map(Out::reborrow));
+                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                None
            }
        }
    }
}

• (another approach would be to error.take() instead of error.as_mut().map(Out::reborrow()); both are equivalent when called only once, such as in your case, but would vary in behavior if write_error() somehow managed to be called multiple times inside a function: with reborrow, the previous Error would be overwritten with no drop glue being run (potential memory leak!), whereas with .take(), the second write would be unable to succeed since it would get a None).

Aside

I don't think you should be repeating the lifetime parameter 'a, for both the duration of the borrow of the message: &str, and for that of the outer Out parameter. Indeed, with that repetition, and with the new &mut Option signature, you'll run into messages of the form &format!(…) being rejected because they "don't live long enough" (messages of the form of "…" string literals, on the other hand, would still be fine):

-   fn report_error<'a>(self, message: &'a str, error: &'_ mut Option<Out<'a, Error>>) -> Option<T> {
+   fn report_error    (self, message: &'_ str, error: &'_ mut Option<Out<'_, Error>>) -> Option<T> {

from safer_ffi.

Thanks for the explanation, this makes sense. There's one problem though, a declaration like

  #[ffi_export]
  fn api_func(mut error: Option<Out<'_, Error>>) -> Option<usize> {

does not compile - I'm getting error underlined and no rules expected this token in macro call error (compiles without mut, but there are obviously other compilation errors telling me to make the parameter mutable). I'm using safer-ffi=0.0.9, is that fixed in the trunk?

Edit: this can be avoided by having a let mut error = error; statement, which is actually what I've been doing previously. (my previous solution used an Option<&'a mut Option<repr_c::Box<Error>>> type, which had some drawbacks, like a double allocation, or a necessity to initialize the error pointer as NULL on the C side)

Edit2: I've tried this solution, and it seems like it doesn't protect from reusing error. That is, I can have

      let x = returns_option().report_error("returns_option() failed", &mut error);
      let y = returns_result().report_error("returns_result() failed", &mut error);

(note the lack of question mark operators), and the compiler will not complain, even though the second call may rewrite the error and leak memory.

from safer_ffi.

The mut error binding got fixed recently: supported as of 0.0.10 (#111)

from safer_ffi.

@fjarri I've moved the discussion and provided more info over the Discussions/ are of this repo: #113

from safer_ffi.