search

dlaidig / vqf

137 stars 24 forks source link

empty custom destructors? #4

Closed kitlith closed 2 years ago

kitlith commented 2 years ago

I noticed that VQF: https://github.com/dlaidig/vqf/blob/694eeb55031382d396f14a6c977c4ad2d940bf66/vqf/cpp/vqf.cpp#L74-L77

and BasicVQF: https://github.com/dlaidig/vqf/blob/694eeb55031382d396f14a6c977c4ad2d940bf66/vqf/cpp/basicvqf.cpp#L45-L48

have explicit empty destructors. Is there a reason for them to be present?

context: i'm writing some basic rust bindings using autocxx. If the custom destructors ware not present, I could mark VQF/BasicVQF as "plain old data" and (in theory) treat them as if they were normal rust types. Otherwise, autocxx assumes that that the destructor is nontrivial, and that rust can not move/drop the type at-will.

dlaidig commented 2 years ago

Cool to see Rust bindings for VQF. :)

I am not quite sure anymore why the empty destructors are there (probably left over from an old version in which I needed them), and I do not see any reason for why they need to stay there. So if removing them helps you, I'm all for it.

Just FYI, the reason for C++98 is to maximize the chance of being compatible with weird microcontroller SDKs...

kitlith commented 2 years ago

That makes sense. I only brought up c++11 just in case you wanted/needed the destructor to be mentioned explicitly. ^-^

Since I pushed the rust bindings i was working on yesterday, have a link: vqf-cxx.

Bindings are incomplete, but basic construction, updating, and obtaining quaternions is available. (I've also only touched the main VQF class, and not BasicVQF/offline vqf)

I'll update the submodule back to the main repo now that the destructors are removed.

dlaidig commented 2 years ago

Thanks for posting the link to the bindings. I'm tempted to test them, but unfortunately I lack the experience and the time to do so in the near future.

When you feel like the bindings are in a state in which they will be useful for most Rust users, feel free to reach out so that we can add a link to the bindings in the documentation. :)