autocxx icon indicating copy to clipboard operation
autocxx copied to clipboard

Could `generate_pod` implement `Clone` and `Copy`?

Open TheButlah opened this issue 2 years ago • 4 comments

Would it make sense for types from generate_pod to implement Copy and Clone?

TheButlah avatar Jun 29 '22 03:06 TheButlah

I'd accept a PR for clone, yep. I am not so sure about copy. I think it's probably best to require explicit cloning.

adetaylor avatar Jun 29 '22 03:06 adetaylor

I don't know enough about the FFI internals to implement this. Would a clone not just memcpy? Or would it have to invoke a c++ copy constructor?

I thought POD types don't have copy constructors

TheButlah avatar Jun 29 '22 03:06 TheButlah

At the moment, we implement a copy constructor for any type which can be copied by implementing moveit's CopyNew trait.

This applies whether the type has an explicit copy constructor or an implicit copy constructor. All of that was insanely complicated because the C++ rules are very precise, @bsilver8192 deserves all the credit.

It's now possible that we could implement Clone in terms of a call to moveit::copy. It's remotely possible we could even do it using a blanket implementation for all types that implement both CopyNew and cxx::private::Trivial, though I suspect we'd need to do codegen for each type. It would be worth trying a blanket implementation first.

But if we need to do the codegen, here's where. There's one mention of CopyNew in engine/src/conversion/analysis/fun/mod.rs. You could alter that code to also spit out a Clone implementation as well as a CopyNew impl.

adetaylor avatar Jun 30 '22 23:06 adetaylor

I don't think Copy works. generate_pod only requires a trivial move constructor and destructor, but Copy is equivalent to a trivial copy constructor and destructor.

unique_ptr/UniquePtr is a good example of a type which can be represented in Rust (ie it has a trivial move constructor and destructor), but cannot be Copy because it has no copy constructor.

Also, a reminder that implicit is not the interesting property to check for. An implicit special member function can be non-trivial. For example, struct X { std::string x; } has an implicit copy constructor, but it is not trivial (because std::string's isn't). I agree with just invoking the C++ copy constructor and letting the compiler figure it out.

I think you could implement conservative checks for trivialness similar to ones I implemented for existence, but it's tricky and autocxx doesn't get enough information from libclang to get it fully correct.

bsilver8192 avatar Jul 02 '22 08:07 bsilver8192