Fix offset_from_ptr_to_memory for axis lengths of 0, and replace it with offset_from_low_addr_ptr_to_logical_ptr #998
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
jturner314
force-pushed
the
fix-offset_from_ptr_to_memory
branch
from
65685ad
to
dd52e27
Compare
bluss
added a commit
to jturner314/ndarray
that referenced
this pull request
May 13, 2021
The first of the added tests failed because of bug rust-ndarray#998, and the fix is verified by the test. The second and third testcases did not have an error, but were added for completeness.
bluss
force-pushed
the
fix-offset_from_ptr_to_memory
branch
from
dd52e27
to
a72c82f
Compare
The first of the added tests failed because of bug rust-ndarray#998, and the fix is verified by the test. The second and third testcases did not have an error, but were added for completeness. The first testcase is by SparrowLii, from the initial mention of the bug.
bluss
force-pushed
the
fix-offset_from_ptr_to_memory
branch
from
a72c82f
to
4ca0f49
Compare
|
Thanks. I pushed in @SparrowLii's testcase + some more. Inserted as the first commit so that I can check it out and verify the failure and then later the fix. |
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.
See SparrowLii's comment regarding the bug fix. Renaming the function, inverting the sign of the return value, and changing the return type to
usizemake the function easier to understand and use, IMO.