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

Add multi_slice_* methods (supports flat tuples only) #717

Merged

Conversation

jturner314
Copy link
Member

@jturner314 jturner314 commented Sep 19, 2019

This is an alternative implementation of #716 that supports only flat tuples of slicing information. This means that it's no longer possible to perform arbitrarily many slices in a single call. The implementations are also limited only to the cases (SliceInfo, SliceInfo, ...) and (&SliceInfo, &SliceInfo, ...), so slicing with e.g. (SliceInfo, &SliceInfo, ...) is not supported.

Despite the fact that the trait implementations are more limited than #716, I prefer this version because the implementation is simpler, it's easier to understand, and the unsafe code is more contained.

The implementation is done, but the tests need improvement.

Fixes #687. Closes #716.

@jturner314 jturner314 force-pushed the multislice-methods-flat-only branch from 1e626f8 to b5da8e0 Compare September 19, 2019 00:36
@jturner314 jturner314 force-pushed the multislice-methods-flat-only branch 2 times, most recently from 22274f6 to 601c1bf Compare September 19, 2019 20:38
@jturner314 jturner314 changed the title Replace multislice! macro with a trait and methods (supports flat tuples only) Add multi_slice_* methods (supports flat tuples only) Sep 19, 2019
/// [`.multi_slice_move()`].
///
/// [`.multi_slice_mut()`]: #method.multi_slice_mut
/// [`.multi_slice_move()`]: type.ArrayViewMut.html#method.multi_slice_move
Copy link
Member

Choose a reason for hiding this comment

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

I don't know about you, but I don't mind transitioning to nightly-only rustdoc links (because they render correctly in docs.rs).. Importance of rendering the links when the user generates docs on stable?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be fine, although I'm having trouble getting nightly-style links to work correctly. The following don't work:

[`.multi_slice_mut()`]
[`multi_slice_mut()`]

If I add an explicit destination, multi_slice_mut works fine, but I'm not sure how to make multi_slice_move work:

This works:
[`.multi_slice_mut()`]

[`.multi_slice_mut()`]: ArrayBase::multi_slice_mut()


This doesn't:
[`.multi_slice_move()`]

[`.multi_slice_move()`]: ArrayViewMut::multi_slice_move()

src/slice.rs Outdated Show resolved Hide resolved
src/slice.rs Outdated Show resolved Hide resolved
src/slice.rs Outdated Show resolved Hide resolved
@jturner314 jturner314 force-pushed the multislice-methods-flat-only branch from 46914be to ef93420 Compare October 15, 2019 00:02
@jturner314 jturner314 merged commit 78846da into rust-ndarray:master Oct 15, 2019
@jturner314 jturner314 deleted the multislice-methods-flat-only branch October 15, 2019 00:39
@jturner314
Copy link
Member Author

Thanks for reviewing this, @bluss. After your approval, I fixed a minor issue in the docs, added more tests, rebased on the latest master, and merged it.

@bluss
Copy link
Member

bluss commented Oct 15, 2019

Great, now we have reason to go for 0.13.1 soon and I can manage the release.

I expect the work after that to be for 0.13.2, further non-breaks.

@bluss
Copy link
Member

bluss commented Apr 11, 2020

I'm sorry that I haven't been around to manage 0.13.1, but it would be ok if others had. That said, maybe I can do it now.

@bluss bluss mentioned this pull request Apr 12, 2020
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.

move semantics for multislice!()
2 participants