sliceAsBytes: include sentinel #19984
Conversation
(This is a reboot of ziglang#18021 after pull request bankruptcy) * `sliceAsBytes()` includes the sentinel value if there is one * fixes 2 bugs the Allocator interface for resize and realloc (they assumed no sentinel before) * cleans up Allocator.free (was manually handling the sentinel) * cleans up resinator (was manually handling the sentinel) Also add `bytesAsSliceSentinel()` for completeness and because I want to use it in [dvui](https://github.com/david-vanderson/dvui) to provide storage for arbitrary user slices.
lib/std/mem.zig
Outdated
| @@ -3835,6 +3835,7 @@ fn CopyPtrAttrs( | |||
| comptime source: type, | |||
| comptime size: std.builtin.Type.Pointer.Size, | |||
| comptime child: type, | |||
| comptime sentinel: ?*const anyopaque, | |||
There was a problem hiding this comment.
Seems to me like this should be comptime sentinel: ?child
There was a problem hiding this comment.
I tried that first, but ran into a bunch of weird errors like error: comptime dereference requires '[:0]u8' to have a well-defined layout (when a sentinel is present), and error: sentinels are only allowed on slices and unknown-length pointers (when no sentinel).
I think there's something about how sentinels are represented at comptime that interferes with that, but I don't understand it. Can you make that work?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes. Sorry I should have been clearer - I tried it originally with the first PR, and I tried it again after @InKryption commented and those are the errors I currently get (when trying comptime sentinel: ?child).
There was a problem hiding this comment.
Now I'm doubting myself - let me recheck.
There was a problem hiding this comment.
Yes I get those errors when trying comptime sentinel: ?child with zig version 0.13.0-dev.211+6a65561e3
There was a problem hiding this comment.
I'm worried this back-and-forth made it unclear - this PR is ready to merge. @InKryption and @rohlem asked if the api could be different, but that would lead to compile errors.
There was a problem hiding this comment.
The compile errors here seem incorrect. Would you be able to make a commit with the proposed change which triggers the compile error, so I can take a look and file a compiler bug if one exists?
There was a problem hiding this comment.
I've committed that change, and it now compiles without error! So this tangent is resolved. Thanks for pushing me to retry it!
|
I'd like to get this into 0.14. Not sure if it warrants a release notes entry, but here's a start:
|
|
I proposed a potential alternative approach here: #22706 (comment), I'd be interested to hear your thoughts on it. |
|
I support #22706 as an alternative to this. |
|
closing in favor of #22706 |
(This is a reboot of #18021 after pull request bankruptcy)
sliceAsBytes()includes the sentinel value if there is oneAlso add
bytesAsSliceSentinel()for completeness and because I want to use it in dvui to provide storage for arbitrary user slices.