lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


Thanks for all the suggestions. Because I do not want to modify metatables for core types or modify the interpreter, it is likely that I will have to revert the behavior some.

To answer your question, Viacheslav Usov, I can indeed protect from the user making that mistake (and I did with my original iteration of type-checker). But it becomes a a problem of violating user expectations in different scenarios. Here are the expectations I'm trying to meet:

User Expectation 1: If something is a nullptr from C or C++, I can check it against "nil" in Lua or put it by itself in an if statement and have it coerce to false.
User Expectation 2: If I push a std::shared_ptr (or std::unique_ptr or boost::shared_ptr), I should be able to retrieve that "std::shared_ptr" out from Lua, even if its a nullptr.

Note that I have no examples of the above expectations showing, in the tutorials or documentation of my library. It is just an assumed expectation that users share. Expectation 1 is far more prevalent than Expectation 2, although both make sense.

Below, I will try to explain as best as I can and muse a bit about the solutions. I will start with a demonstration:

std::shared_ptr<int> giving_function ( ) {
     return nullptr;
}

void receiving_function (std::shared_ptr<int>& s) {
     // do something with s
}

lua_State* L = luaL_newstate();
// binding magic to insert giving_function into the state
// binding magic to insert receiving_function into the state
const char* code = "
     v = giving_function() 
     print(v == nil)
     receiving_function(v) 
";

luaL_dostring(L, code);

The way I handled it at first with the "binding magic" was this:
1. check if the pointer is null
2a. Yes -> push Lua's nil with `lua_pushnil(L)` (what sol::nil represents in C++ land)
2b. No -> push userdata

With this way, doing a check for "v == nil" works and the code prints "true". But, when "receiving_function" is called and it inspects "v"s type, there's a bit of a problem. What's being asked for is a reference to a std::shared_ptr. This does not work because lua_touserdata returns `nullptr`, and thusly there's no std::shared_ptr inside I can pass a reference to. The type checker did fail this code. Users complained that this code should "still work", because it violated their expectations. Someone filed an issue, where I gave them a tiny amount of advice to just take a raw pointer T*, where I could convert Lua's nil to a "nullptr" and not have to find a "std::shared_ptr" inside of the userdata.

Still, I tried a second way to handle it with "binding magic":
1. push userdata of std::shared_ptr, regardless of whether it contained a nullptr or not

With this way, doing a check for "v == nil" does not work and prints "false". This violated user expectations and once more an issue was filed with my library. When "receiving_function" is called and it inspects "v"s type, it works because there's a userdata, and thusly I can hand the user a reference to the std::shared_ptr, even if that std::shared_ptr is just a container for "nullptr". Note: A fringe benefit of doing it the secondary way is if these userdata are put into a table as a "sequence", then there are no "nil" holes. (This does not affect collections and data structures returned from C++, as we override the metatable for them and let Lua 5.2/5.3 proper iterator or pairs functions, where we use the well-designed Lua iteration hooks to provide safe iteration over the entire sequence or data structure regardless of what it contains.)

"But why does someone need a reference to the std::shared_ptr and not just a value?"
When we store non-primitive types in Lua with our "binding magic" (sol2), they are stored as userdata of some sort. All userdata carry with it the expectation that you can take a reference to the original data that you pushed: this allows people to do things like work with "std::unique_ptr"s and other types which cannot be copied (and thusly, we cannot take them by-value in a function because "by-value" in a function signature means "take a copy"). It also allows someone to directly manipulate the data held in Lua, without needing to perform a special dance or copy, swap or move their data (all operations can be done in-place on Lua's memory). I also cannot specialize the behavior for checking if someone takes a shared_ptr by-value versus if they ask for a reference: I always have to return a reference from the function that retrieves a specific type.

"What about writing a null-checking function?"
I have spent a long-time curating the Binding Magic (sol2) Library's documentation. Nobody will read it unless something goes wrong, and even then I need to find a way to allow them to quickly find out that working with sol2's types would require me to find the perfect spots to place this information so nobody makes any mistakes. I want the library to provide good defaults that make the most sense, which is why I'm trying these ways and exploring options for handling it. If I write a null-checking function, I also need to make it opt-in. I have a mechanism for including "built-in" libraries, and I could very well write a "built in" lib that comes with my framework where people have to use "sol.is_null( some_userdata )" to check. This, introduces a piece of sol2-specific code into people's code-bases and inevitably ties them to me. The goal of this library is that all of the code looks and feels like Lua code, with no special library-internal libs or strings attached. The Lua code is supposed to look like plain old Lua code, and be replaceable with Lua metatables and other stuff so it works as seamlessly as possible.

"What about Tamás Kiss explanation of using a NULL userdata or a table to do == comparisons with?"
Again, I would need to introduce this special NULL type to the user and make it something they have to opt-into to use (like with the null-checking function). I can do it and it gets closer to elegant by enabling the syntax "if some_userdata == sol.null", but once more I'm injecting Library-specific types that other people's code now depends on (and many would not know specifically how to implement). I would like to avoid features that lay-users would have a hard time replicating on their own.

"So then maybe returning actual Lua nil is the best solution, and you just need to warn people who use unique pointer-types to be careful?"
I am beginning to think this is the best way to go. People have depended on me doing things the first-way for quite a long while now, and this recent change was an experiment to see if things would work out properly. The "there's a hole in my sequence table" problem is not really a problem I care for either, because -- as I mentioned before -- with C++ containers as userdata I can simply override the default iteration ability. If someone is deliberately sticking with Lua 5.1 -- for LuaJIT or other reasons -- then they expect/deserve the pain of ipairs/pairs and its non-overridable behavior for old, crufty 5.1. I only dream that someday LuaJIT will release a 5.3-complaint version of itself, with all the squeaky, shiny metatable wheels and the lack of table-enforcing checks in pairs/ipairs, and summarily kill of the usage of Lua 5.1 except for a few niche users.

But that is just a dream. :)

I apologize for the long-winded explanation, but I wanted to explain everything in-full. Thanks to everyone who helped me bikeshed this: if there's no other keen suggestions I will likely revert to just doing `lua_pushnil` when I detect a "nullptr", and warn users accordingly that not taking a raw pointer (T*) or not taking a raw reference (T&) of the underlying type is a dangerous game with Lua, since Lua owns the memory and directly modifying the internally-stored unique pointer types is bad.

Sincerely,
ThePhD


On Fri, Jun 30, 2017 at 6:35 AM, Viacheslav Usov <via.usov@gmail.com> wrote:
On Thu, Jun 29, 2017 at 6:51 PM, ThePhD <jm3689@columbia.edu> wrote:


I have tried to understand your problem, but something does not click.

What I have understood is that when the user code wants to push a shared_ptr holding a nullptr, your code pushes sol::nil, which is not Lua's nil. Correct so far?

Then the user pops something assuming it is a shared_ptr pushing earlier, and your code also assumes it is shared_ptr, just casting it as such. But in this case it casts sol::nil as shared_ptr, and that is bad. Is that correct?

The disconnect I have here is why, since you know that in certain cases you push shared_ptr as sol::nil, the popping code has no check for this case. That sounds like a very silly bug, easily fixable, so I must be missing something.

Cheers,
V.