| author |  Mitchell Hashimoto <mitchell.hashimoto@gmail.com> 2022-03-16 12:21:48-07:00 |
| committer |  Andrew Kelley <andrew@ziglang.org> 2022-03-16 16:26:54-07:00 |
| log | 418197b6c51b99ee2a0be5e394753868f7dbf982 |
| tree | 0cd552ad21afc0d61827a8ad248ef50e2cbbaa85 |
| parent | 94672dfb1941289eb65fdeab2e1dcc39ca70c3b7 |
stage2: elem_ptr needs to know if slice or direct access
This fixes one of the major issues plaguing the `std.sort` comptime tests.
The high level issue is that at comptime, we need to know whether `elem_ptr` is
being used to subslice an array-like pointer or access a child value. High-level
example:
var x: [2][2]i32 = undefined;
var a = &x[0]; // elem_ptr, type *[2]i32
var y: [5]i32 = undefined;
var b = y[1..3]; // elem_ptr, type *[2]i32
`a` is pointing directly to the 0th element of `x`. But `b` is
subslicing the 1st and 2nd element of `y`. At runtime with a well
defined memory layout, this is an inconsequential detail. At comptime,
the values aren't laid out exactly in-memory so we need to know the
difference.
This becomes an issue specifically in this case:
var c: []i32 = a;
var d: []i32 = b;
When converting the `*[N]T` to `[]T` we need to know what array to point
to. For runtime, its all the same. For comptime, we need to know if its
the parent array or the child value.
See the behavior tests for more details.
This commit fixes this by adding a boolean to track this on the
`elem_ptr`. We can't just immediately deref the child for `&x[0]`
because it is legal to ptrCast it to a many-pointer, do arithmetic, and
then cast it back (see behavior test) so we need to retain access to the
"parent" indexable.3 files changed, 96 insertions(+), 2 deletions(-)
test- behavior
src/Sema.zig+1-1
| ... | @@ -16739,7 +16739,7 @@ fn elemPtrArray( | ... | @@ -16739,7 +16739,7 @@ fn elemPtrArray( |
| 16739 | const index_u64 = index_val.toUnsignedInt(); | 16739 | const index_u64 = index_val.toUnsignedInt(); |
| 16740 | // @intCast here because it would have been impossible to construct a value that | 16740 | // @intCast here because it would have been impossible to construct a value that |
| 16741 | // required a larger index. | 16741 | // required a larger index. |
| 16742 | const elem_ptr = try array_ptr_val.elemPtr(array_ptr_ty, sema.arena, @intCast(usize, index_u64)); | 16742 | const elem_ptr = try array_ptr_val.elemPtrDirect(array_ptr_ty, sema.arena, @intCast(usize, index_u64)); |
| 16743 | return sema.addConstant(result_ty, elem_ptr); | 16743 | return sema.addConstant(result_ty, elem_ptr); |
| 16744 | } | 16744 | } |
| 16745 | } | 16745 | } |
src/value.zig+27-1
| ... | @@ -505,6 +505,7 @@ pub const Value = extern union { | ... | @@ -505,6 +505,7 @@ pub const Value = extern union { |
| 505 | .array_ptr = try payload.data.array_ptr.copy(arena), | 505 | .array_ptr = try payload.data.array_ptr.copy(arena), |
| 506 | .elem_ty = try payload.data.elem_ty.copy(arena), | 506 | .elem_ty = try payload.data.elem_ty.copy(arena), |
| 507 | .index = payload.data.index, | 507 | .index = payload.data.index, |
| 508 | .direct = payload.data.direct, | ||
| 508 | }, | 509 | }, |
| 509 | }; | 510 | }; |
| 510 | return Value{ .ptr_otherwise = &new_payload.base }; | 511 | return Value{ .ptr_otherwise = &new_payload.base }; |
| ... | @@ -2402,7 +2403,11 @@ pub const Value = extern union { | ... | @@ -2402,7 +2403,11 @@ pub const Value = extern union { |
| 2402 | .decl_ref_mut => return val.castTag(.decl_ref_mut).?.data.decl.val.elemValueAdvanced(index, arena, buffer), | 2403 | .decl_ref_mut => return val.castTag(.decl_ref_mut).?.data.decl.val.elemValueAdvanced(index, arena, buffer), |
| 2403 | .elem_ptr => { | 2404 | .elem_ptr => { |
| 2404 | const data = val.castTag(.elem_ptr).?.data; | 2405 | const data = val.castTag(.elem_ptr).?.data; |
| 2405 | return data.array_ptr.elemValueAdvanced(index + data.index, arena, buffer); | 2406 | if (!data.direct) |
| 2407 | return data.array_ptr.elemValueAdvanced(index + data.index, arena, buffer); | ||
| 2408 | |||
| 2409 | const underlying = try data.array_ptr.elemValueAdvanced(data.index, arena, buffer); | ||
| 2410 | return underlying.elemValueAdvanced(index, arena, buffer); | ||
| 2406 | }, | 2411 | }, |
| 2407 | 2412 | ||
| 2408 | // The child type of arrays which have only one possible value need | 2413 | // The child type of arrays which have only one possible value need |
| ... | @@ -2465,12 +2470,25 @@ pub const Value = extern union { | ... | @@ -2465,12 +2470,25 @@ pub const Value = extern union { |
| 2465 | 2470 | ||
| 2466 | /// Returns a pointer to the element value at the index. | 2471 | /// Returns a pointer to the element value at the index. |
| 2467 | pub fn elemPtr(val: Value, ty: Type, arena: Allocator, index: usize) Allocator.Error!Value { | 2472 | pub fn elemPtr(val: Value, ty: Type, arena: Allocator, index: usize) Allocator.Error!Value { |
| 2473 | return val.elemPtrAdvanced(ty, arena, index, false); | ||
| 2474 | } | ||
| 2475 | |||
| 2476 | /// Returns a pointer to the element value at the index. The behavior | ||
| 2477 | /// of this is slightly different for comptime; the "direct" means that | ||
| 2478 | /// indexing indexes the referenced child value, not the parent array. | ||
| 2479 | pub fn elemPtrDirect(val: Value, ty: Type, arena: Allocator, index: usize) Allocator.Error!Value { | ||
| 2480 | return val.elemPtrAdvanced(ty, arena, index, true); | ||
| 2481 | } | ||
| 2482 | |||
| 2483 | pub fn elemPtrAdvanced(val: Value, ty: Type, arena: Allocator, index: usize, direct: bool) Allocator.Error!Value { | ||
| 2468 | const elem_ty = ty.elemType2(); | 2484 | const elem_ty = ty.elemType2(); |
| 2469 | const ptr_val = switch (val.tag()) { | 2485 | const ptr_val = switch (val.tag()) { |
| 2470 | .slice => val.castTag(.slice).?.data.ptr, | 2486 | .slice => val.castTag(.slice).?.data.ptr, |
| 2471 | else => val, | 2487 | else => val, |
| 2472 | }; | 2488 | }; |
| 2473 | 2489 | ||
| 2490 | // If the val is already an elem ptr, then we do ptr arithmetic logic | ||
| 2491 | // and just move the index. | ||
| 2474 | if (ptr_val.tag() == .elem_ptr) { | 2492 | if (ptr_val.tag() == .elem_ptr) { |
| 2475 | const elem_ptr = ptr_val.castTag(.elem_ptr).?.data; | 2493 | const elem_ptr = ptr_val.castTag(.elem_ptr).?.data; |
| 2476 | if (elem_ptr.elem_ty.eql(elem_ty)) { | 2494 | if (elem_ptr.elem_ty.eql(elem_ty)) { |
| ... | @@ -2478,6 +2496,12 @@ pub const Value = extern union { | ... | @@ -2478,6 +2496,12 @@ pub const Value = extern union { |
| 2478 | .array_ptr = elem_ptr.array_ptr, | 2496 | .array_ptr = elem_ptr.array_ptr, |
| 2479 | .elem_ty = elem_ptr.elem_ty, | 2497 | .elem_ty = elem_ptr.elem_ty, |
| 2480 | .index = elem_ptr.index + index, | 2498 | .index = elem_ptr.index + index, |
| 2499 | |||
| 2500 | // Retain the direct preference. This enables a direct | ||
| 2501 | // elem ptr (i.e. &arr[0]) to be bitcasted to a many-pointer | ||
| 2502 | // with pointer arithmetic then casted back to a single | ||
| 2503 | // pointer. | ||
| 2504 | .direct = elem_ptr.direct, | ||
| 2481 | }); | 2505 | }); |
| 2482 | } | 2506 | } |
| 2483 | } | 2507 | } |
| ... | @@ -2485,6 +2509,7 @@ pub const Value = extern union { | ... | @@ -2485,6 +2509,7 @@ pub const Value = extern union { |
| 2485 | .array_ptr = ptr_val, | 2509 | .array_ptr = ptr_val, |
| 2486 | .elem_ty = elem_ty, | 2510 | .elem_ty = elem_ty, |
| 2487 | .index = index, | 2511 | .index = index, |
| 2512 | .direct = direct, | ||
| 2488 | }); | 2513 | }); |
| 2489 | } | 2514 | } |
| 2490 | 2515 | ||
| ... | @@ -4194,6 +4219,7 @@ pub const Value = extern union { | ... | @@ -4194,6 +4219,7 @@ pub const Value = extern union { |
| 4194 | array_ptr: Value, | 4219 | array_ptr: Value, |
| 4195 | elem_ty: Type, | 4220 | elem_ty: Type, |
| 4196 | index: usize, | 4221 | index: usize, |
| 4222 | direct: bool, | ||
| 4197 | }, | 4223 | }, |
| 4198 | }; | 4224 | }; |
| 4199 | 4225 |
test/behavior/pointers.zig+68
| ... | @@ -437,3 +437,71 @@ test "indexing array with sentinel returns correct type" { | ... | @@ -437,3 +437,71 @@ test "indexing array with sentinel returns correct type" { |
| 437 | var s: [:0]const u8 = "abc"; | 437 | var s: [:0]const u8 = "abc"; |
| 438 | try testing.expectEqualSlices(u8, "*const u8", @typeName(@TypeOf(&s[0]))); | 438 | try testing.expectEqualSlices(u8, "*const u8", @typeName(@TypeOf(&s[0]))); |
| 439 | } | 439 | } |
| 440 | |||
| 441 | test "element pointer to slice" { | ||
| 442 | if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO | ||
| 443 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO | ||
| 444 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO | ||
| 445 | |||
| 446 | const S = struct { | ||
| 447 | fn doTheTest() !void { | ||
| 448 | var cases: [2][2]i32 = [_][2]i32{ | ||
| 449 | [_]i32{ 0, 1 }, | ||
| 450 | [_]i32{ 2, 3 }, | ||
| 451 | }; | ||
| 452 | |||
| 453 | const items: []i32 = &cases[0]; // *[2]i32 | ||
| 454 | try testing.expect(items.len == 2); | ||
| 455 | try testing.expect(items[1] == 1); | ||
| 456 | try testing.expect(items[0] == 0); | ||
| 457 | } | ||
| 458 | }; | ||
| 459 | |||
| 460 | try S.doTheTest(); | ||
| 461 | comptime try S.doTheTest(); | ||
| 462 | } | ||
| 463 | |||
| 464 | test "element pointer arithmetic to slice" { | ||
| 465 | if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO | ||
| 466 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO | ||
| 467 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO | ||
| 468 | |||
| 469 | const S = struct { | ||
| 470 | fn doTheTest() !void { | ||
| 471 | var cases: [2][2]i32 = [_][2]i32{ | ||
| 472 | [_]i32{ 0, 1 }, | ||
| 473 | [_]i32{ 2, 3 }, | ||
| 474 | }; | ||
| 475 | |||
| 476 | const elem_ptr = &cases[0]; // *[2]i32 | ||
| 477 | const many = @ptrCast([*][2]i32, elem_ptr); | ||
| 478 | const many_elem = @ptrCast(*[2]i32, &many[1]); | ||
| 479 | const items: []i32 = many_elem; | ||
| 480 | try testing.expect(items.len == 2); | ||
| 481 | try testing.expect(items[1] == 3); | ||
| 482 | try testing.expect(items[0] == 2); | ||
| 483 | } | ||
| 484 | }; | ||
| 485 | |||
| 486 | try S.doTheTest(); | ||
| 487 | comptime try S.doTheTest(); | ||
| 488 | } | ||
| 489 | |||
| 490 | test "array slicing to slice" { | ||
| 491 | if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO | ||
| 492 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO | ||
| 493 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO | ||
| 494 | |||
| 495 | const S = struct { | ||
| 496 | fn doTheTest() !void { | ||
| 497 | var str: [5]i32 = [_]i32{ 1, 2, 3, 4, 5 }; | ||
| 498 | var sub: *[2]i32 = str[1..3]; | ||
| 499 | var slice: []i32 = sub; // used to cause failures | ||
| 500 | try testing.expect(slice.len == 2); | ||
| 501 | try testing.expect(slice[0] == 2); | ||
| 502 | } | ||
| 503 | }; | ||
| 504 | |||
| 505 | try S.doTheTest(); | ||
| 506 | comptime try S.doTheTest(); | ||
| 507 | } |