Skip to content

mem.sliceTo on C pointers should return an optional slice #8785

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

Closed

Conversation

daurnimator
Copy link
Contributor

https://github.com/ziglang/zig/pull/8698/files#r632922210

Also makes the compile errors from SliceTo more consistent

@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label May 15, 2021
@daurnimator daurnimator requested a review from ifreund May 15, 2021 09:50
Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure this makes sense with the current semantics of C pointers, if #6597 were accepted I would think otherwise.

If this is the way to go, I think we need to update std.mem.span() to match this behavior, and you need to correct the doc comment for std.mem.sliceTo().

@daurnimator daurnimator force-pushed the sliceToOnNullCPointer branch from 43c6b13 to 61f97cf Compare May 15, 2021 10:28
@daurnimator
Copy link
Contributor Author

If this is the way to go, I think we need to update std.mem.span() to match this behavior,

hmm. I think I'd only like to do that if #6597 is accepted.

you need to correct the doc comment for std.mem.sliceTo().

done.

@ifreund
Copy link
Member

ifreund commented May 15, 2021

hmm. I think I'd only like to do that if #6597 is accepted.

I think having consistent behavior between the two is pretty important, what's your reasoning here?

@daurnimator
Copy link
Contributor Author

I think having consistent behavior between the two is pretty important, what's your reasoning here?

span looks like it shouldn't accept C pointers at all; and users should instead be using sliceTo.
Rather than change it, I'd rather remove the .C branch entirely.

@daurnimator daurnimator force-pushed the sliceToOnNullCPointer branch from 61f97cf to 632239f Compare May 15, 2021 11:57
@andrewrk
Copy link
Member

Please re-open against master branch with passing tests

@andrewrk andrewrk closed this Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants