Make iterators covariant in element type #1417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bluss
force-pushed
the
baseiter-covariant
branch
from
c165929
to
75b5f93
Compare
The internal Baseiter type underlies most of the ndarray iterators, and it used `*mut A` for element type A. Update it to use `NonNull<A>` which behaves identically except it's guaranteed to be non-null and is covariant w.r.t the parameter A. Add compile test from the issue. Fixes #1290
bluss
force-pushed
the
baseiter-covariant
branch
from
75b5f93
to
a9605dc
Compare
bluss
commented
Aug 6, 2024
| @@ -907,7 +907,7 @@ where D: Dimension | |||
|
|
|||
| // iter is a raw pointer iterator traversing the array in memory order now with the | |||
| // sorted axes. | |||
| let mut iter = Baseiter::new(self_.ptr.as_ptr(), self_.dim, self_.strides); | |||
| let mut iter = Baseiter::new(self_.ptr, self_.dim, self_.strides); | |||
There was a problem hiding this comment.
Review note: it's evident that Baseiter is constructed from a NonNull everywhere, which means that its non-null requirement is easily fulfilled.
Complete the transition to using NonNull as the raw pointer type by using it as Baseiter's iterator element type.
bluss
force-pushed
the
baseiter-covariant
branch
from
901a998
to
00e1546
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The internal Baseiter type underlies most of the ndarray iterators, and it used
*mut Afor element type A. Update it to useNonNull<A>which behaves identically except it's guaranteed to be non-null and is covariant w.r.t the parameter A.Add compile test from the issue.
The second commit continues the "conversion", using NonNull more consistently
in Baseiter's Iterator impl.
Fixes #1290