allow getting at the Lua state from a Lua object #161
Conversation
hlua/src/lib.rs
Outdated
| /// higher-level APIs. For example, pushing a value onto the Lua stack will cause `PushGuard`s | ||
| /// in Rust code to be out of sync with the Lua stack. | ||
| #[inline] | ||
| pub fn state_ptr(&self) -> *mut ffi::lua_State { |
There was a problem hiding this comment.
This shouldn't return a *mut ffi::lua_State, otherwise people are going to run into semver issues.
Instead I think it should return a *mut c_void, where c_void is imported from std::os::raw::c_void.
There was a problem hiding this comment.
Hmm, by "semver issues" do you mean: say hlua moves to lua52-sys 0.2, but the user's using lua52-sys 0.1?
If so then I think this is still preferable to returning a void*. For a void*, to do anything useful a cast is always necessary. For a ffi::lua_State pointer a cast is only necessary if the version has changed.
(I might be misunderstanding what you're saying -- in that case feel free to correct me!)
There was a problem hiding this comment.
Per some discussion here, an alternate solution would be to export the entire ffi module. That way callers that want to use the version of lua52-sys that corresponds to the hlua in use can just use it through hlua::ffi.
What do you think?
In tomaka#161 there was some concern that exporting a pointer to the underlying Lua state directly would cause semver issues. This neatly bypasses those issues.
In tomaka#161 there was some concern that exporting a pointer to the underlying Lua state directly would cause semver issues. This neatly bypasses those issues.
|
Hey :) not sure if you saw it, but I pushed updated commits. Let me know what you think, thanks. |
|
I'm really cautious about doing this. |
|
I do actually need the change, yeah. I'm implementing a library on top of
this that adds support for interacting with Lua coroutines. I need to
perform a number of low-level operations to make that work. At the moment
I'm relying on mem::transmute from a LuaContext, but that's not great.
Could you elaborate a bit on what worries you about this?
…On Aug 31, 2017 08:18, "tomaka" ***@***.***> wrote:
I'm really cautious about doing this.
Do you actually need the change, or are you just submitting this PR
because "why not"?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#161 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AALBijYqaOPXZNtfBvsdEMBsslZA4fu9ks5sds7GgaJpZM4PE_Fv>
.
|
|
In my opinion if you need to call raw Lua functions, it means that hlua is missing the functionality you need. It also means that it could be dangerous in the future to change the internal behaviour of hlua (eg. change so that some function pushes two elements on the stack instead of one), because it may break code that uses the FFI. |
|
Yeah -- completely agreed that it's not a feature most regular users will
want to use. I do think that it's mitigated somewhat by the fact that to do
anything at all with the ffi you'll need to use unsafe.
My other changes like allowing creation of PushGuards have a similar
problem -- if not used carefully, they could cause Rust and Lua to be out
of sync. That's why I made those functions unsafe.
But consider that with these changes in place, I was able to build on top
of your excellent work and get Lua coroutines working for my use case in
just a couple of days and a few hundred lines of code. Without hlua's
support I'd still be reimplementing function calls, poorly. 🙂
I'd be happy to put in warnings/documentation in appropriate places to warn
regular users off.
…On Aug 31, 2017 10:56, "tomaka" ***@***.***> wrote:
In my opinion if you need to call raw Lua functions, it means that hlua is
missing the functionality you need.
Exposing the ffi kind of shows that the library fails at its goal.
It also means that it could be dangerous in the future to change the
internal behaviour of hlua (eg. change so that some function pushes two
elements on the stack instead of one), because it may break code that uses
the FFI.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#161 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AALBilT71cMX8XktHAf-bc3Bh2jPN7Jmks5sdvPWgaJpZM4PE_Fv>
.
|
|
Hi @tomaka -- did you come to a discussion here? In the worst case I guess I'll keep on using |
|
Since I'm still undecided, please add |
In tomaka#161 there was some concern that exporting a pointer to the underlying Lua state directly would cause semver issues. This neatly bypasses those issues.
This is most useful when the caller wants to do low-level operations on the state that aren't supported through the main API. Also add a test to verify that low-level access works.
|
Done! Thanks so much for your time dealing with this. (If you could get a release out after this lands, that would be awesome! I can get rid of all our hacks at that point. :) ) |
This is most useful when the caller wants to do low-level operations on
the state that aren't supported through the main API.