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

Simplifications for slicing-related types #940

Merged
merged 4 commits into from
Mar 15, 2021

Conversation

jturner314
Copy link
Member

I realized that now that we no longer have a slicing API based on the Dimension::SliceArg associated type, the s![] macro can return an owned type, the tricky conversions involving pointer casts no longer need to exist, and SliceInfo no longer needs to be ?Sized.

This PR does the following:

  • Implement SliceArg<D> for &T.
  • Change the slicing methods to take I rather than &I.
  • Make the s![] macro return an owned SliceInfo rather than a reference.
  • Replace the conversions of SliceInfo involving tricky pointer casts with similar conversions not involving pointer casts.
  • Make SliceInfo be Sized and remove the #[repr(transparent)] attribute. These changes aren't necessary, but seem like a nice simplification.

This allows the `s![]` macro to return an owned type, without
requiring an explicit `&` when calling slicing methods.
The original reason for `SliceInfo` being `?Sized` and these
conversions was to work around the limitations of the slicing methods
taking a type involving `Dimension::SliceArg`. Now that the slicing
methods take `I: SliceArg<D>` as the parameter type, these conversions
are no longer necessary, and `SliceInfo` doesn't need to be `?Sized`.
@bluss bluss merged commit 2bcbb2a into rust-ndarray:master Mar 15, 2021
@bluss
Copy link
Member

bluss commented Mar 15, 2021

Nice!

@bluss bluss added this to the 0.15.0 milestone Mar 15, 2021
@jturner314 jturner314 deleted the make-sliceinfo-sized branch March 15, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants