WiresmithTech / Rust-LabVIEW-Interop

A crate for creating libraries in Rust that are called from LabVIEW
MIT License
7 stars 4 forks source link

procedural derive macro for LV FFI Error Handling #35

Open jkneer opened 3 weeks ago

jkneer commented 3 weeks ago

Best practice in LV is to wrap functionality into an error handling case structure and have error ins and outs into all VIs.

We could add the same thing automatically to DLL functions with a derive macro.

I am working on a with_context kind of macro/closure API for the error handling. But ultimately the boiler plate for this can be greatly reduced by converting this into a procedural macro:

Normal macro with boilerplate:

macro_rules! with_lverrorhandling {
    // Match a variadic number of parameters
    ($error_cluster:expr, $func:expr, $($params:expr),*) => {
        {
            // only run if the incoming error cluster does not have an error
            if !$error_cluster.status() {
            // Call the function within a context
                match $func($($params),*) {
                    Ok(_) => LVStatusCode::SUCCESS,
                    Err(err) => {
                        let errstr = err.to_string();
                        let errcode = LVStatusCode::from(err);

                        // Update the error cluster if there's an error
                        if let Ok(cluster) = unsafe { $error_cluster.as_ref_mut() } {

                            cluster.set_error(errcode, "Rust Interop Error", &errstr).unwrap();
                        }
                        // Return the appropriate LVStatusCode
                        errcode
                    }
                }
            } else {
                $error_cluster.code()
            }
        }
    };
}

Function with macro applied:

#[cfg(target_pointer_width = "64")]
#[no_mangle]
pub extern "C" fn auto_lv_errorhandling(
    error_cluster: ErrorClusterPtr,
    param1: isize,
    param2: isize,
) -> LVStatusCode {
    use labview_interop::errors::LVInteropError;

    with_lverrorhandling!(
        &error_cluster,
        |p1, p2| {

            // DO stuff, use ?, etc to deal with errors in rust, the context macro takes care
            // of updating the error cluster

            Err::<(), LVInteropError>(LVInteropError::InternalError(
                labview_interop::errors::InternalError::ArrayDimensionMismatch,
            ))
        },
        param1,
        param2
    )
}

With a procedural macro errorhandled_externc this could be simplified to

#[cfg(target_pointer_width = "64")]
#[derive(errorhandled_externc)]
pub fn auto_lv_errorhandling(
    param1: isize,
    param2: isize,
) -> Result<()> {
            // DO stuff, use ?, etc to deal with errors in rust, the context macro takes care
            // of updating the error cluster
}
jkneer commented 2 weeks ago

https://github.com/jkneer/Rust-LabVIEW-Interop/tree/johannes/error-handling has the declarative macro... a procedural macro is definitely the way to go to avoid all the boilerplate code.

#[no_mangle]
pub extern "C" fn set_x_mode_by_id(
    error: ErrorClusterPtr,
    id: LStrHandle,
    lv_x: UPtr<i32>,
) -> LVStatusCode {
    with_lverrorhandling!(
        &error,
        |id1: LStrHandle, lv_x1: UPtr<i32>| -> Result<()> {
            let id = id1.to_rust_string();
            let x = Xode::try_from_primitive(*lv_x1)?;

            with_mut_device(&id, |device, _is_live| {
                info!("Setting X mode {:?}", x);
                device.set_x_mode(x)?;
                Ok(())
            })
        },
        id,
        lv_x
    )
}
JamesMc86 commented 1 week ago

I think this looks interesting but really needs splitting out from the existing PR else we won't ever get it merged!

jkneer commented 1 week ago

Yes, this is in a separate branch. I'm using that in some my code and it's really quite nice to have, but still a lot of boilerplate.

I can create a PR with just this macro. The procedural macro would then be a separate crate, and PR, anyways.

JamesMc86 commented 4 days ago

Just reviewing through this. It looks like a good idea, I've been essentially writing some boilerplate anyway.

A couple of thoughts on the examples you've shown:

I love the idea though as I've been using this pattern myself a lot now. Now my DLLs give me error messages instead of crashing out!

jkneer commented 3 days ago

My initial reaction is to avoid the derive macro. I think it is better to see the exact call parameters so it is easy to compare the configuration in LabVIEW for errors. That said, it is much cleaner.

Currently rust-annalyzer does a bad job with code in the macro call. Any syntax error will just highlight the full closure/macro body which renders the error highlighting mostly useless. I resorted to writing the closure separately. Overall I found it less annoying than writing the full boilerplate, but still quite annoying.

Can we move the parameters to before the function in the macro? I'm guessing this could be a limitation of the macro syntax.

My thought was that the closure being defined in the body would already have the parameters, even with type in that position and just having them twice seemed annoying. Also, I'm not sure if the macro syntax allows for a parameter to follow the variadic parameters.

I would not have the code in the return position as well. That may be useful for returning parameters.

I thought this way was reducing the complexity by removing the option of the returning parameter being something else - at the cost of flexibility. It was a decision in the heat of the moment, tbh, I have no preference for one or the other.

I presume we can make this generic so the internal function could return either of our error types or something with source information or additional details.

My Idea was that the closure should take any impl ToLvError as error. Can that be expressed in the macro syntax?

JamesMc86 commented 3 days ago

I was just thinking about this, it is a closure so it can capture the parameters anyway, no need for them to be handled in the macro.

It actually could just be a function I think, it wouldn't need to be a macro (though I'm not against it).

fn labview_error_handling(error_cluster: ErrorClusterPtr, routine: impl FnOnce() -> impl ToLvError) {
  let lv_error = routine().into();
  // update error cluster
}

Or even as a function on the error cluster ptr type?