Extend Rust FFI #3450
Extend Rust FFI #3450
Conversation
|
Also serves as bisect for #3439 |
Ericson2314
force-pushed
the
more-rust-ffi
branch
from
f650dde to
233e672
Compare
Do idiomatic C++ copy and move constructors for a few things, so wrapping structs' defaults can work.
Ericson2314
force-pushed
the
more-rust-ffi
branch
from
d32877e to
e433d4a
Compare
|
|
||
| void operator = (const StorePath & that) | ||
| { | ||
| (rust::Value<3 * sizeof(void *) + 24, ffi_StorePath_drop>::operator = (that)); |
There was a problem hiding this comment.
Where does that magic size come from? I see that we start to duplicate that throughout (at least) this file a few times. Shouldn't there be a comment where the magic 24 comes from? For the 3 * sizeof(void *) part one can probably guess that the struct hold 3 pointers. The 24 bytes could be 3 integers? Can't we use a single sizeof(SomeStruct) instead? Does this take into account alignment changes if a new member is added to whatever the underlying struct is?
I know you didn't introduce it but this might go out of sync with the other definition thus I am commenting.
There was a problem hiding this comment.
Nevermind the numbers, I am repeating the whole superclass. Maybe I should #ifdef and then #undef the superclass; that would help without leaking anything more to the files the include the header.
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
|
@edolstra It has come up again in #3673 and #3523 that the lack of copy constructors this PR fixes prevents us from using idiomatic C++ STL functions, which is a bummer. Is there anything I can do to make this acceptable for you? For example, after #3671 is merged, I plan on writing additional unit tests to ensure |
|
To someday return to if a Rust rewrite happens. |
|
Relevant: https://github.com/dtolnay/cxx |
Do idiomatic C++ copy and move constructors for a few things, so
wrapping structs' defaults can work.