Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crystal callback #15

Open
veelenga opened this issue Apr 24, 2017 · 20 comments · May be fixed by #38
Open

Crystal callback #15

veelenga opened this issue Apr 24, 2017 · 20 comments · May be fixed by #38

Comments

@veelenga
Copy link
Owner

Would be nice to be able to write function callback in Crystal.

@veelenga
Copy link
Owner Author

Currently it is possible to do the following:

stack = Lua::Stack.new

proc = ->(state : LibLua::State) {
  puts LibLua.tonumberx(state, 1, nil)
  return 0
}

# push crystal proc onto the stack
LibLua.pushcclosure(stack.state, proc, 0)

# give it a name
LibLua.setglobal(stack.state, "crystal")

# call crystal function with an argument. 
stack.run "crystal(10)" #=> 10.0

stack.close

@zatherz
Copy link

zatherz commented Jul 3, 2017

Bump

@veelenga
Copy link
Owner Author

veelenga commented Jul 4, 2017

@zatherz sorry, I don't have so much time for this now. Will try to work on it during coming weeks. If you are able to prepare PR I would love to review/merge it ;) Any other help also appreciated...

@zatherz
Copy link

zatherz commented Jul 4, 2017

I was going to write a PR, but I'm not decided on the API. Should the proc receive a low level state like above or should it get an Array? Array of what? How should assigning it to a global work?

@veelenga
Copy link
Owner Author

veelenga commented Jul 4, 2017

Well, the example above is low level. For convenience lua.cr is designed to be more high level. The API I am thinking about is the following:

lua = Lua.load

# define a new crystal callback/function
lua.function "crystal_add" do |x, y|
  x + y
end

# use it in Lua
lua.eval %q{
  return 100 * crystal_add(3, 4)
} #=> 700.0

Here is the same that is written in a low-level manner:

lua = Lua.load

proc = ->(state : LibLua::State) {
  x = LibLua.tonumberx(state, 1, nil)
  y = LibLua.tonumberx(state, 2, nil)

  # push result to stack
  LibLua.pushnumber(state, x + y)

  1 # number of results
}

# push crystal proc onto the stack
LibLua.pushcclosure(lua.state, proc, 0)

# give it a name
LibLua.setglobal(lua.state, "crystal_add")

# call crystal function with arguments.
pp lua.run %q{
  return 100 * crystal_add(3.0, 4.0)
} #=> 700.0

lua.close

So, basically, to achieve this, you need to create new method function on Stack (via new mixin) that will accept the function name and a block and do the work from the low-level example:

  1. Having the number of block arguments, fetch them from the stack.
  2. Call the block with those arguments and save the result. Note that Crystal can return only one argument as a function/proc result.
  3. Push result back onto the stack

@veelenga
Copy link
Owner Author

veelenga commented Jul 4, 2017

You can try to start and create PR on early stage, so we can discuss it if needed.

PS: I was highly inspired by ruby's version of this: rufus-lua. That may help you a lot.

Thanks for being interested in this project ;)

@zatherz
Copy link

zatherz commented Jul 4, 2017

What is the type of x and y in

lua.function "crystal_add" do |x, y|
  x + y
end

?

@veelenga
Copy link
Owner Author

veelenga commented Jul 4, 2017

Hm, that has to be (Bool | Float64 | Lua::Object | String | Nil), a union type returned while doing pop, so yes, it will require casting before doing +.

@zatherz
Copy link

zatherz commented Jul 6, 2017

That should be an alias, but with what name? Lua::Any? Maybe lua.function should look like this:

lua.function "crystal_add", Float64, Float64 do |x, y|
  x + y
end

?
This would ensure that the arguments have those types using the standard luaL_argcheck function, and would make binding to Crystal a lot easier.

@veelenga
Copy link
Owner Author

veelenga commented Jul 6, 2017

Yes, there is an alias for this, Lua::Type.

Regarding lua.function, good suggestion. I was thinking about similar approaches too. But might be hard to work with if there are lots of arguments. I think as the prototype it can be implemented without type checks and extended later. IMO this has to be easier to do...

@bew
Copy link
Contributor

bew commented Dec 12, 2017

Maybe using the Proc notation like this would be interesting too:

lua.function "crystal_add", ->(x : Float64, y : Float64) do
  x + y
end

This would easily allow passing arbitrary procs like:

def foo(x)
  x + 1
end

lua.function "cr_foo", ->foo(Int32)

int = 42
lua.function "increment_int_from_lua", ->int.succ
lua.eval "increment_int_from_lua()"
puts int # => 43

@veelenga
Copy link
Owner Author

@bew good suggestion. I have to return to this and implement it ;) But if you are willing to work on it, do not hesitate to open a PR 😼

@bew
Copy link
Contributor

bew commented Dec 12, 2017

looks like I need to use a macro to properly find arguments names & types of the passed proc, and properly generate the Lua proc (taking a Lua state, and working with that).

Maybe the API could look like:

def foo(x)
  x + 1
end

lua.function "cr_foo", Lua.make_function ->foo(Int32)

int = 42
lua_function_proc = Lua.make_function ->int.succ
lua.function "increment_int_from_lua", lua_function_proc
lua.eval "increment_int_from_lua()"
puts int # => 43

What do you think?

Edit: we'll need crystal-lang/crystal#5206 to be merged, to be able to inspect the ProcPointer (->foo) and ProcLiteral (->{ 1 }) nodes

@veelenga
Copy link
Owner Author

veelenga commented Dec 12, 2017

Sorry, I might need to look at it closer, but does it make sense to create Lua.make_function macro, if you can mix macro stuff in lua.function? So your first suggestion should also work:

lua.function "cr_foo", ->foo(Int32)

I am talking about it abstractly, so may be wrong.

BTW, why do you interested in this? I abandoned it a bit because haven't seen practical examples/usage...

@bew
Copy link
Contributor

bew commented Dec 12, 2017

While you can use macros inside a function, you cannot access its arguments as AST nodes, and inspect the properties of these AST nodes, for example, here we need to get the Int32 in ->foo(Int32).

Given this simplified implementation:

def function(name, proc)
  lua_proc = ->(state : LibLua::State) do
    # /!\ --------> Not possible, as `proc` is nothing here in macro world
    {% for arg in proc.args %}
      # extract the argument from the stack, based on the type of `arg`
    {% end %}
    # call `proc` with the fetched arguments from the stack
    # put back the result in the stack
    1
  end

  # associate the lua_proc with a global name
  # etc..
end

That's why I think the macro is necessary.

BTW reply: nothing special, I just really like Lua, and found this project interesting for scripting inside a crystal app, and I think Lua deserve a cool library to run Lua scripts in Crystal 🌟

@veelenga
Copy link
Owner Author

That's correct. But I was just saying that function may be a macro instead.

@bew
Copy link
Contributor

bew commented Dec 13, 2017

Ah that's not possible with current Crystal, as macros cannot be called on instances (for good IMO), so lua.function cannot be a macro, only a method. Macros can only be called in top level, in a namespace (module or class) or preceded by its namepace (like Some::Path.some_macro)

@shayneoneill
Copy link

Was any progress made on this? I've got a pretty important use for it, and the Proc stuffs a bit goofy

@veelenga
Copy link
Owner Author

veelenga commented Aug 12, 2018

@shayneoneill no progress. And I can't promise you it will be done in the nearest future.

jwoertink added a commit to jwoertink/lua.cr that referenced this issue Aug 29, 2021
@jwoertink jwoertink linked a pull request Aug 29, 2021 that will close this issue
@jwoertink
Copy link

I just pushed up a WIP PR for this. I don't really know what I'm doing, but if anyone wants to take a look and give a little guidance, I'll fix it up. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants