Should TensorType be a sealed trait or marked unsafe?

[ammaraskar]

trafficstars

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that the TensorType trait is commented as:

Currently, all implementors must not implement Drop (or transitively contain anything that does) and must be bit-for-bit compatible with the corresponding C type. Clients must not implement this trait.

Since implementing this trait improperly potentially allows for dangerous behavior and is commented as something that shouldn't be implemented, would it make sense to seal this trait as per https://rust-lang.github.io/api-guidelines/future-proofing.html#sealed-traits-protect-against-downstream-implementations-c-sealed or to mark it an unsafe trait?

[adamcrume]

It should definitely be sealed. This would be a great easy improvement for someone in the community to work on.

[eaeiv]

I would like this issue to be assigned to me. I believe I understand the sealed paradigm and have it implemented properly within lib.rs. Not 100% sure the proper way to test its correctness though.

The only real decision I'm not sure about is to impl Sealed for every individual type rust_type and q_type utilizes (bool + all ints and floats under 64), or just impl <T> Sealed for T {} to generalize the trait.

[eaeiv]

@adamcrume I have the change finished and documented. Can I be assigned this issue so I can create the pull request?

I decided on the templated approach as the underlying trait is templated without restrictions.

[adamcrume]

Definitely! I look forward to seeing the PR.