Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove generic_const_exprs #368

Merged
merged 4 commits into from
Oct 20, 2023
Merged

Conversation

calebzulawski
Copy link
Member

Inspired by the errors in #367 (caused by rust-lang/rust#116320) I've removed all instances of generic_const_exprs.

For ToBitMaskArray, this was straightforward, simply use an associated type instead of an associated const. For the various to_bytes functions, this was more complicated. I now implemented a ToBytes trait over every individual vector type. In the future when const generics are more powerful, I'd hope this can be implemented generically.

This effectively upgrades these functions to nightly, as they're currently disabled on core.

/// The length of the bitmask array.
const BYTES: usize;
/// The bitmask array.
type BitMaskArray;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should have some useful bounds, such as Copy + Unpin + Send + Sync + 'static + AsRef<[u8]> + AsMut<[u8]> + BorrowMut<[u8]> and maybe more

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would adding more bounds in the future be backwards compatible?

Copy link
Member

@workingjubilee workingjubilee Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, bounds can only be relaxed backwards-compatibly, and only sometimes. They cannot be added, so forward-compatibility requires maximum bounds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that still true when the trait is sealed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...that does change things a bit, since the major compat hazard is people referencing the type generically.

/// Convert SIMD vectors to vectors of bytes
pub trait ToBytes: Sealed {
/// This type, reinterpreted as bytes.
type Bytes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use some bounds here too.

crates/core_simd/src/to_bytes.rs Outdated Show resolved Hide resolved
@calebzulawski calebzulawski merged commit 3e4e13c into master Oct 20, 2023
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants