Relax requirements for UserData to impl FromLua

[jonas-schievink]

This was only allowed for UserData implementors that are also Copy.
This relaxes the requirement to be Clone instead.

While Copy makes sense to prevent allocations and other potentially
costly operations, other FromLua impls already do pretty expensive
stuff, so this seems worth it.

[kyren]

Yeah, I definitely agree that the style of this API is to allow some pretty expensive stuff to happen for the sake of usability. I'm not disagreeing with you, in fact I think this change is probably worth it, but I feel a bit guilty about it if it means implicitly calling a user defined clone. Maybe it's really obvious what would be going on and hard to be bitten by? I feel bad about some of the overhead of things like to/from conversion and table iteration particularly, and I was planning on making a real performance push at some point, but the current API design limits what I could accomplish somewhat. The problem is, I spent a LONG time trying to come up with an API that would, for example, allow me to eschew registry operations and the like, and it's just very very very hard to design a consistent API that looks different than this and isn't unsafe, for some definition of unsafe (actually unsound, or just trivial to cause panics accidentally and hard to use).

I guess I'm sort of making excuses for the overhead in the API, but in an ideal world where, for example table iteration was practically as fast as what you'd unsafely write in C, you could make the argument that all those allocating from conversions SHOULDN'T exist, and they should instead be more explicit. By virtue of trying to make a library that was flexible and hard to misuse, I think I'm sort of being led down the opposite path of making everything expensive and convenient. Still, having those from impls certainly makes things like 'HashSet<String, String>' vastly easier to deal with. Also to impls will always obviously allocate, so why not from impls?

I'm interested in your thoughts on this.

[jonas-schievink]

The way I see most impls of ToLua and FromLua is just as a convenience when you either must convert Rust<->Lua data structures to glue 2 APIs together, or when you just don't need high performance and want the convenience of Rust's data structures.

It might be possible to mimic a big part of Rust's Vec / *Map API for Table (where Vec functions only concern the table's sequence part and ignore the rest, like we discussed before), and I think that could lead to a quite usable API (well, of course there's no way to get rid of the lifetime parameter). If we had that, just using Table would be as ergonomic as using Rust's native data structures. Maybe in the future we could come up with a design that splits expensive conversions from cheap ones, requiring explicit opt-in to use the expensive ones.

An orthogonal idea I had: Implement FromLua for std::cell::Ref<T> where T: UserData (and RefMut). This is cheap and does automatic type/borrow checking, but feels a bit... unwieldy.

[kyren]

It might be possible to mimic a big part of Rust's Vec / *Map API for Table (where Vec functions only concern the table's sequence part and ignore the rest, like we discussed before), and I think that could lead to a quite usable API (well, of course there's no way to get rid of the lifetime parameter). If we had that, just using Table would be as ergonomic as using Rust's native data structures. Maybe in the future we could come up with a design that splits expensive conversions from cheap ones, requiring explicit opt-in to use the expensive ones.

Yeah, I definitely think we're thinking along the same lines here

An orthogonal idea I had: Implement FromLua for std::cell::Ref where T: UserData (and RefMut). This is cheap and does automatic type/borrow checking, but feels a bit... unwieldy.

You know, that's actually not a bad idea, though it does feel unwieldy. What it does though is remove the Clone that you may not want and remove the awkward extra step of unwrapping the UserData, especially when you accept the converted value as an argument in the hypothetical future improved API.

In any case, I think you're right that with the way the API currently works, this is a good change.