This repository was archived by the owner on Mar 17, 2026. It is now read-only.
Repository navigation
This repository was archived by the owner on Mar 17, 2026. It is now read-only.
Should TensorType be a sealed trait or marked unsafe? #284
Description
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?
Activity
adamcrume commentedon Jul 7, 2021
It should definitely be sealed. This would be a great easy improvement for someone in the community to work on.
eaeiv commentedon Jul 11, 2022
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 forevery individual type rust_type and q_type utilizes (bool + all ints and floats under 64), or justimpl <T> Sealed for T {}to generalize the trait.eaeiv commentedon Jul 13, 2022
@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 commentedon Jul 14, 2022
Definitely! I look forward to seeing the PR.
Class Serialisation Performance Maintenance (tensorflow#284)