Sema: allow @ptrCast of slices changing the length

[mlugg]

Also, refactor Sema.ptrCastFull to avoid emitting AIR int_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?

[ifreund]

How does this handle sentinel terminated slices? From the code I think this PR matches the current behavior of mem.sliceAsBytes() when, for example, casting a [:0]u32 to a []u8.

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.

[mlugg]

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:

• [:0]u8 to []u8
• [:0]u8 to []i8
• [:0]u32 to []u8

The first one doesn't even require a @ptrCast, but currently this is just equivalent to dropping the sentinel, and I think that's clearly the sane thing in this case. The cast explicitly says "I am not interested in this sentinel".

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, @as([]u8, @ptrCast(some_slice)) would include the sentinel of some_slice unless it was a slice of u8. So, I think we would clearly need to go all-or-nothing here, and "all" seems incredibly unintuitive to me.

This doesn't mean sliceAsBytes should necessarily do the same.

[ifreund]

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.

Yes, after further consideration I tend to agree that it makes the most sense to always use foo.len (which is defined to exclude the sentinel) for the calculations involved in this cast.

The reason I ask is that @andrewrk expressed hope that we could delete mem.sliceAsBytes()/mem.bytesAsSlice() entirely after this feature lands, which I also think would be a reasonable thing to do.

I'm not sure it would be a good idea to have the semantics of @ptrCast() and mem.sliceAsBytes() differ subtly here.

Perhaps a better alternative would be to add a mem.absorbSentinel() function which, when passed a [:0]T returns a []T with len incremented by 1. I think this would be quite readable without being overly verbose:

const foo: [:0]u32 = whatever;
const bytes: []const u8 = @ptrCast(mem.absorbSentinel(foo));

[david-vanderson]

I agree with both of you, and dropping mem.sliceAsBytes()/mem.bytesAsSlice() sounds good. @ifreund is right - we need something to include the sentinel if there is one (name maybe mem.includeSentinel()). For dvui this would be a clear improvement for storing/retrieving arbitrary slices.

Based on my experience with #19984 there will be bugs with people forgetting to include mem.includeSentinel(), but I can't think of a better solution.

[rohlem]

there will be bugs with people forgetting to include mem.includeSentinel(), but I can't think of a better solution.

Fwiw, one potential solution would be to make dropping the sentinel require an explicit slicing operation slice[0..len:] / slice[0..:] / slice[:].
This behavior was brought up as a footgun in #17668, this solution was suggested in #17668 (comment),
and then the idea was rejected and the issue closed (imo without convincing reasoning; I'd still like this change in the language).

[andrewrk]

skipping release notes, you know what to do if you want to add them