Version bump to 1.19 caused compilation errors

[sgpthomas]

The type changes to Engine::register_fn caused a new compilation error. I had some code where I rust couldn't infer the types for register_fn and so I had to specify them explicitly. The change to the types caused our code to mis-compile. It was trivial to fix, and the new types look more ergonomic, but I thought you should know.

[sgpthomas]

To add a touch more detail, Rust can't figure out the R: Variant + Clone parameter. Not sure if there is a way to structure the types so that this is easier to infer. But maybe something worth looking into.

[schungx]

Can I ask from which version you upgraded from? The function registration interface hasn't changed for quite a while, so I suppose you're upgrading from a much lower version?

I do understand that, sometimes, when registering a function with a closure that returns Result, I'd need to manually specify the error type for it to pass... But so far you're the first user to comment that it breaks code.

[sgpthomas]

I was updating from 1.18 -> 1.19. This was the commit that changed things: https://github.com/rhaiscript/rhai/commit/f7728f778e7f045b05b57424736eabe5500369a5. In particular removing the generic parameter and replacing it with an impl RhaiNativeFunc<..>. Removing the generic parameter meant that any code that explicitly instantiated that parameter, no longer compiled.

[schungx]

I think I understand what you're saying but can you show a minimal snippet?

[sgpthomas]

Sure. Here is the code we had in 1.18

self.engine.register_fn::<_, 4, true, OpRef, true, _>(...);

and here is the code for 1.19

self.engine.register_fn::<_, 4, true, OpRef, true>(...);

We had to remove the last underscore for it to compile. Previously this underscore represented the type of the closure as RhaiNativeFunc<..>. In 1.19, that generic was removed and replaced with the closure argument having type impl RhaiNativeFunc<..>. Removing the generic argument breaks any code that needs to explicitly pass these type parameters.

[schungx]

Ah. I got you now.

I was under the impression that most users would simply have Rust infer the generic parameters so this change is not breaking for most.

May I ask why you need to specify generic parameters? Why can't Rust figure it out?

What if you omit the generic parameters altogether?

[sgpthomas]

Here's the error message:

error[E0283]: type annotations needed
   --> fud2/fud-core/src/script/plugin.rs:211:21
    |
211 |         self.engine.register_fn(
    |                     ^^^^^^^^^^^ cannot infer type for type parameter `R` declared on the method `register_fn`
    |
    = note: cannot satisfy `_: Variant`
note: required by a bound in `rhai::api::register::<impl Engine>::register_fn`
   --> ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rhai-1.19.0/src/api/register.rs:69:12
    |
65  |     pub fn register_fn<
    |            ----------- required by a bound in this associated function
...
69  |         R: Variant + Clone,
    |            ^^^^^^^ required by this bound in `rhai::api::register::<impl Engine>::register_fn`

error[E0283]: type annotations needed
   --> fud2/fud-core/src/script/plugin.rs:211:21
    |
211 |         self.engine.register_fn(
    |                     ^^^^^^^^^^^ cannot infer type for closure `{closure@fud2/fud-core/src/script/plugin.rs:213:13: 218:38}`
    |
    = note: multiple `impl`s satisfying `{closure@fud2/fud-core/src/script/plugin.rs:213:13: 218:38}: RhaiNativeFunc<_, _, _, _, _>` found in the `rhai` crate:
            - impl<FN, R, S, T, U, V, RET> RhaiNativeFunc<(R, S, T, U, V), 5, true, RET, false> for FN
              where for<'a> <FN as FnOnce<(NativeCallContext<'a>, R, S, T, U, V)>>::Output == RET, for<'a> FN: Fn(NativeCallContext<'a>, R, S, T, U, V), FN: rhai::func::native::SendSync, FN: 'static, R: Variant, R: Clone, S: Variant, S: Clone, T: Variant, T: Clone, U: Variant, U: Clone, V: Variant, V: Clone, RET: Variant, RET: Clone;
            - impl<FN, R, S, T, U, V, RET> RhaiNativeFunc<(R, S, T, U, V), 5, true, RET, true> for FN
              where for<'a> <FN as FnOnce<(NativeCallContext<'a>, R, S, T, U, V)>>::Output == Result<RET, Box<EvalAltResult>>, for<'a> FN: Fn(NativeCallContext<'a>, R, S, T, U, V), FN: rhai::func::native::SendSync, FN: 'static, R: Variant, R: Clone, S: Variant, S: Clone, T: Variant, T: Clone, U: Variant, U: Clone, V: Variant, V: Clone, RET: Variant, RET: Clone;
note: required by a bound in `rhai::api::register::<impl Engine>::register_fn`
   --> ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rhai-1.19.0/src/api/register.rs:74:20
    |
65  |     pub fn register_fn<
    |            ----------- required by a bound in this associated function
...
74  |         func: impl RhaiNativeFunc<A, N, X, R, F> + SendSync + 'static,
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `rhai::api::register::<impl Engine>::register_fn`

For more information about this error, try `rustc --explain E0283`.
error: could not compile `fud-core` (lib) due to 2 previous errors

It for some reason can't figure out what the return type of the function is supposed to be. From a little playing around, it seems to be because I'm returning a Result type from this function, so that I can propagate errors.

Here's an example where Rust fails to infer the return type.

engine.register_fn("test", |input: &str| Ok(0))

Adding the types explicitly works (to verify that this closure can be a RhaiNativeFunc):

engine.register_fn::<_, 1, false, usize, true>("test", |_input: &str| Ok(0));

Explicitly specifying the Result type in the closure allows Rust to figure out the types.

this.engine.register_fn("test", |_input: &str| {
    let r: Result<usize, Box<EvalAltResult>> = Ok(0);
    r
});

Which implies, I think, that Rust type inference breaks down when it has to infer both the return of RhaiNativeFunc and the error type of the Result. I have no idea how you might change the types to make this inference possible.

[schungx]

Ah. Now I understand. Yes, this is the problem with type detection.

The problem usually is with not specifying the error type instead of the return type. The compiler cannot figure out the error type... it knows about the return type.

Can you try:

engine.register_fn("test", |input: &str| -> Result<_, Box<EvalAltResult>> { Ok(0) })

or

pub type RhaiResult<T> = Result<T, Box<EvalAltResult>>;

engine.register_fn("test", |input: &str| -> RhaiResult<_> { Ok(0) })

[schungx]

I quite liberally add/remove generic parameters to public functions with the hope that nobody really specify them...

That's why I try to use impl Trait as much as possible to avoid users specifying the types specifically.

Otherwise it is really restrictive and nothing can be done on public API's without bumping to a new major version.

[sgpthomas]

That makes sense. But it is technically a breaking change. Maybe it would be helpful to ad a note on the readme / in the documentation, saying that you don't intend type parameters to be stable and avoid using them as much as possible.

[schungx]

I already have API's that are specifically marked as Volatile or Low Level so the user using those should expect breakage...

However, register_fn is a very normal function which is not volatile.