-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Sema: allow @ptrCast
of slices changing the length
#22706
Conversation
10fee1c
to
62b4abf
Compare
How does this handle sentinel terminated slices? From the code I think this PR matches the current behavior of However, I think #19984 makes a strong case that it is more useful and less bug-prone to include the reinterpreted memory of the sentinel in the resulting slice. |
I think it makes sense for the builtin to exclude sentinels. As is, Zig-the-language never implicitly moves a sentinel to a slice's main length; I think that's a reasonable property to maintain. In fact, the behaviour you propose would be incredibly weird. Consider these cases:
The first one doesn't even require a The second one is a slight variation on the first which can no longer be done via simple coercion. Again, it would be incredibly surprising if it included the sentinel in the result length. The last one is the kind of thing this PR enables. If it included the sentinel while the other two didn't, the behavior would feel incredibly inconsistent; for instance, This doesn't mean |
Yes, after further consideration I tend to agree that it makes the most sense to always use The reason I ask is that @andrewrk expressed hope that we could delete I'm not sure it would be a good idea to have the semantics of Perhaps a better alternative would be to add a const foo: [:0]u32 = whatever;
const bytes: []const u8 = @ptrCast(mem.absorbSentinel(foo)); |
I agree with both of you, and dropping Based on my experience with #19984 there will be bugs with people forgetting to include |
Fwiw, one potential solution would be to make dropping the sentinel require an explicit slicing operation |
Also, refactor `Sema.ptrCastFull` to not be a horrifying hellscape.
Also, refactor
Sema.ptrCastFull
to avoid emitting AIRint_from_ptr
on slices. That was actually the original point of this branch :P--
cc @jacobly0 -- double check that you're okay with the new ptrcast AIR?