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

empty slice should still assign ptr properly #4771

Open
BarabasGitHub opened this issue Mar 21, 2020 · 5 comments
Open

empty slice should still assign ptr properly #4771

BarabasGitHub opened this issue Mar 21, 2020 · 5 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@BarabasGitHub
Copy link
Contributor

BarabasGitHub commented Mar 21, 2020

Zig version: 0.5.0+153c6cf92

const testing = @import("std").testing;
test "empty slice ptr" {
    const a : []u8 = try testing.allocator.alloc(u8, 10);
    defer testing.allocator.free(a);
    var b : []u8 = a[0..0];
    var c : []u8 = undefined;
    c = a[0..0];
    testing.expectEqual(a.ptr, b.ptr); // works
    testing.expectEqual(a.ptr, c.ptr); // fails
}

It's also works fine using an array to initialize the first slice somehow:

const testing = @import("std").testing;
test "empty slice ptr" {
    const a : []u8 = &[_]u8{1,2,3,4};
    var b : []u8 = a[0..0];
    var c : []u8 = undefined;
    c = a[0..0];
    testing.expectEqual(a.ptr, b.ptr);
    testing.expectEqual(a.ptr, c.ptr);
}

My git bisect said: 2182d28cb0917b8d869d13802a5955ee35b4537a is the first bad commit

@BarabasGitHub
Copy link
Contributor Author

My current workaround is to assign to ptr and len explicitly. (Kinda surprised that's allowed, but ok.)

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 28, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Mar 28, 2020
@andrewrk andrewrk changed the title Empty slice doesn't assign ptr properly empty slice should still assign ptr properly Mar 28, 2020
@andrewrk
Copy link
Member

This was a language decision that consciously changed in that commit. But let's make it a bit more formal, by at least having this be a proposal to revert the language semantics.

@BarabasGitHub
Copy link
Contributor Author

What's the behaviour supposed to be then?
Because currently it only in a very specific case where it doesn't assign the ptr property as I'd expect.

In my use case I have multiple slices. One with the full memory block and another with the part that's actually in use. Sometimes it has to be reset to be empty which is done with [0..0], which is the only time it ends up with an incorrect value. (Actually in the init function where it starts with some capacity.) It's very surprising to me and I don't immediately see how it makes sense. So yes I'd like to propose to revert to the old behaviour of always assigning the pointer when I set a slice, whether it's with a comptime known empty array or not.

@daurnimator
Copy link
Collaborator

This bit me when I had an ArrayList with 0 items (but large capacity): the .ptr field of the slice got set to null.

@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
ianprime0509 added a commit to ianprime0509/zig-libgit2 that referenced this issue Aug 13, 2022
libgit2 allows the `parents` pointer to be null if there are 0 parents,
and Zig appears to set `ptr` to null for zero-length slices:
ziglang/zig#4771
ianprime0509 added a commit to ianprime0509/zig-libgit2 that referenced this issue Aug 17, 2022
libgit2 allows the `parents` pointer to be null if there are 0 parents,
and Zig may set `ptr` to null for zero-length slices:
ziglang/zig#4771
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants
@andrewrk @daurnimator @BarabasGitHub and others