Deregister exeternal value

[tychedelia]

There are some (unsafe) values that I need to insert into the engine that have the lifetime of a single script execution. In order to maintain my invariants, I would like to be able to deregister an external value, so that I can remove it from the engine after my script completes.

[mattwparas]

Thanks for opening this. There is an API that exists for passing scoped references in to the engine that removes the value afterwards, however I don't think there is anything for value types at the moment that matches that API. Could you provide an example of the API you're looking for? Here is an example using the scoped reference API that already exists (forgive the scratch, I'm away from my computer so can't get the exact example yet):

/// somewhere there is something like "impl custom reference" for Context

                engine.with_mut_reference::<Context, Context>(cx).consume(
                      move |engine, arguments| {
                          let context = arguments[0].clone();
                          engine.update_value("*helix.cx*", context);

                          engine.call_function_by_name_with_args(name, args.clone())
                      },
                  )

[tychedelia]

There is an API that exists for passing scoped references

Thanks, this API looks helpful and is generally the pattern I want. Unfortunately, I'm working with external types which complicates things a bit. Right now, the proc macro for Custom doesn't handle lifetimes, so it's difficult to create a newtype wrapper for my foreign reference I'm handed in my function. I'm using a newtype around raw pointers to get around this, which works fine with register_external_value, but seems like the same pattern that with_mut_reference is doing internally anyway. While this scoped solution may work for me, I still think it might be nice to have a way to remove values from the engine, for handling objects which might not have a well defined lifetime.

[mattwparas]

I agree with what you've said - let me try to outline how I think you could use the existing API to suit your needs, and I'll also investigate what it would take to do otherwise:

struct Context<'a>(&'a TypeNameHere);

impl<'a> CustomReference for Context<'a> {}

steel::custom_reference!(Context<'a>);

fn test(cx: &mut Context) {
  engine.with_mut_reference::<Context, Context>(cx).consume(
      move |engine, arguments| {
          let context = arguments[0].clone();
          // Update value so the same variable is used
          engine.update_value("*helix.cx*", context);

          engine.call_function_by_name_with_args(name, args.clone())
      },
  )
}

[tychedelia]

A few issues I'm running into. Here's my code:

#[derive(Debug, Steel, Clone)]
struct WorldHolder<'w>(UnsafeWorldCell<'w>);

impl <'w> CustomReference for WorldHolder<'w> {}

steel::custom_reference!(WorldHolder<'a>);

1:

error[E0726]: implicit elided lifetime not allowed here
  --> src/script/mod.rs:33:8
   |
33 | struct WorldHolder<'w>(UnsafeWorldCell<'w>);
   |        ^^^^^^^^^^^ expected lifetime parameter
   |
help: indicate the anonymous lifetime
   |
33 | struct WorldHolder<'_><'w>(UnsafeWorldCell<'w>);

Not sure why the proc maco is causing the compiler to output this invalid <'_><'w> syntax.

2: Much more minor ergonomic issue,steel::custom_reference requires using the hard-coded 'a lifetime.

I'm also not totally in love with the API. The purepose of the dual type params isn't totally clear from the call site, and it seems like the user will almost always want to call consume immediately which makes it a bit more verbose. Ideally, I'd like to have something like this

engine.with_mut_reference("foo", foo, |engine| {
   // do whatever
});

where the call to create the scope would register the variable itself to reduce some of the boilerplate.

Btw, really impressed with this crate and looking forward to using it more. I've been dying for an embedded lisp in Rust and was really encouraged to see this project despite whatever rough edges it may still have.

[mattwparas]

Thank you for the kind words, I appreciate it 😄

You're right, the dual type params are confusing and I don't like it either. I haven't yet figured out how to only specify the one, and this point it will just require some more thinking on the API and trying some things out to make the type checker happy. You're also right on the macro, it does need the 'a lifetime. I'll look into making that generic. Its a rather rough part of the API that I have only stress tested a bit. Of course, documentation needs to be improved as well.

You're also spot on with the API - that API is used when passing multiple references, there is a happy path that exists already that does exactly what you want I believe:

// This a method on `Engine`

    pub fn run_thunk_with_reference<
        'a,
        'b: 'a,
        T: CustomReference + 'b,
        EXT: CustomReference + 'static,
    >(
        &'a mut self,
        obj: &'a mut T,
        mut thunk: impl FnMut(&mut Engine, SteelVal) -> Result<SteelVal>,
    ) -> Result<SteelVal>
    where
        T: ReferenceMarker<'b, Static = EXT>,
    {
        self.with_mut_reference(obj).consume(|engine, args| {
            let mut args = args.into_iter();

            thunk(engine, args.into_iter().next().unwrap())
        })
    }

Per the issue you're having - can you try removing the #[derive(Steel)]? If it still has issues compiling that I'll run some tests in the morning, but I know that will probably bark if it has lifetimes in the definition (which is where that error might be coming from)

[tychedelia]

Per the issue you're having - can you try removing the #[derive(Steel)]? I

Unfortunately, when I remove Steel, I'm no longer able to use my value as input to my functions. For a little context, basically every function I'm defining on the Rust side takes a context object World as it's first argument. I'm writing in the context of a game engine, and so am provided a reference to &mut World on every tick, which I then need to inject into my scripts that run on that frame.

There might be a bit of an x/y problem here in terms of what I'm describing. In most other scripting engines I've used, functions on the Rust side are defined something like fn(ctx: Context, args: Arguments), where Context is a handle to the engine. For example, if I were writing in JavaScript, I would stick my World object into the current executions realm in a "host defined" variable, where I could then pull it out in the body of my Rust functions without having to explicitly pass it in on the script side.

This kind of signature / pattern is much less ergonomic for writing Rust glue, since it requires pulling values out of an untyped arguments object, checking for arity, etc. I honestly really like how convenient it is to write Steel functions in Rust with POD that "just work." I'm also not totally bothered by passing my context object as the first argument to everything, since this is a very "lispy" kind of pattern anyway.

But yeah, that's a bit more color and elaboration on what I'm trying to accomplish. :)

Edit: I should also say the current examples use a once_cell mutex approach to this problem, which is totally cool and works! This is more about ergonomics.

[mattwparas]

So I might be missing something, however I'm not sure why you're no longer able to use the value as the input to your functions. Could you be more specific about how it is not working? It should still work as defined, where the first argument can be an &mut Context or something that you can manipulate. For example, maybe seeing how I've integrated steel into another project:

https://github.com/mattwparas/helix/blob/steel-event-system/helix-term/src/commands/engine/steel.rs

For example, this function accepts a context and can just be registered into a module no problem:
https://github.com/mattwparas/helix/blob/steel-event-system/helix-term/src/commands/engine/steel.rs#L2081

It might be just an oversight in the API as it is now that its not working for you right now.

Edit: The custom_reference! should remove the necessity for the Custom derive

[tychedelia]

Thanks, this is a good reference. I like your solution for templating functions that hide passing your context object. 👍

Here's the error I'm getting when I remove the Steel derive:

error[E0277]: the trait bound `Engine: RegisterFn<for<'a, 'b> fn(&'a WorldHolder<'b>, std::string::String) -> std::option::Option<script::EntityRef> {op}, _, _>` is not satisfied
  --> src/script/mod.rs:72:40
   |
72 |                     .register_fn("op", op)
   |                      -----------       ^^ the trait `RegisterFn<for<'a, 'b> fn(&'a WorldHolder<'b>, std::string::String) -> std::option::Option<script::EntityRef> {op}, _, _>` is not implemented for `Engine`
   |                      |
   |                      required by a bound introduced by this call
   |
   = help: the following other types implement trait `RegisterFn<FN, ARGS, RET>`:
             <Engine as RegisterFn<FN, steel::steel_vm::register_fn::Wrapper<()>, RET>>
             <Engine as RegisterFn<FN, steel::steel_vm::register_fn::Wrapper<(A,)>, RET>>
             <Engine as RegisterFn<FN, steel::steel_vm::register_fn::Wrapper<(A, B)>, RET>>
             <Engine as RegisterFn<FN, AsyncWrapper<()>, RET>>
             <Engine as RegisterFn<FN, AsyncWrapper<(A,)>, RET>>
             <Engine as RegisterFn<FN, MarkerWrapper1<(A, B)>, RET>>
             <Engine as RegisterFn<FN, MarkerWrapper1<SELF>, RET>>
             <Engine as RegisterFn<FN, MarkerWrapper2<(A, B)>, RET>>
           and 41 others

Here's the relevant code:

#[derive(Debug, Clone)]
struct WorldHolder<'w>(UnsafeWorldCell<'w>);

impl <'w> CustomReference for WorldHolder<'w> {}

steel::custom_reference!(WorldHolder<'a>);

#[derive(Debug, Steel, PartialEq, Clone)]
pub struct EntityRef(Entity);

fn op(world: &WorldHolder, name: String) -> Option<EntityRef> {
  None
}

[mattwparas]

aha, this is almost assuredly just a short coming with the register function api (which I'm happy to fix) - can you try making that an &mut reference to the world?

[tychedelia]

can you try making that an &mut reference to the world?

Ah! Great. Everything is working now. Yeah, in practice it's fine for these to have exclusive access, but I'd like to be able to enforce read only access for getters vs setters, etc. I'm going to read through your Helix implementation some more, but this is working for now. :)

[mattwparas]

Awesome - yeah so this whole api was certainly gnarly to implement and as you can probably tell, I implemented it so that I could get the helix API to work nicely (and more or less, stopped there). There are almost assuredly things I've missed and nuances that I didn't cover. Feel free to open issues / continue here / do whatever with use cases if something doesn't work, happy to add more to this

Realistically you'll probably try to add a function soon and you think it'll work and it will just say "absolutely not" with some horribly complex compiler message. If that occurs, the fastest way I can fix it is to just hit me with the function signature you're trying to add and I'll see why it doesn't work (and most likely, add the patch quickly that will make it work) 😄