| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-03 07:54:53+00:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-03 22:28:37+00:00 |
| log | b4da8eef2a393543e9520c544364689ab482b080 |
| tree | 3c21ba65a2bcb5eb3d970868d6d8b08e3757a073 |
| parent | 252c2031011f80b35d01d5ba5a2a2577c870f90c |
Zir: split up start and end of range in `for_len`
The old lowering was kind of neat, but it unintentionally allowed the
syntax `for (123) |_| { ... }`, and there wasn't really a way to fix
that. So, instead, we include both the start and the end of the range in
the `for_len` instruction (each operand to `for` now has *two* entries
in this multi-op instruction). This slightly increases the size of ZIR
for loops of predominantly indexables, but the difference is small
enough that it's not worth complicating ZIR to try and fix it.4 files changed, 42 insertions(+), 42 deletions(-)
lib/std/zig/AstGen.zig+13-15
| ... | @@ -7020,7 +7020,7 @@ fn forExpr( | ... | @@ -7020,7 +7020,7 @@ fn forExpr( |
| 7020 | const indexables = try gpa.alloc(Zir.Inst.Ref, for_full.ast.inputs.len); | 7020 | const indexables = try gpa.alloc(Zir.Inst.Ref, for_full.ast.inputs.len); |
| 7021 | defer gpa.free(indexables); | 7021 | defer gpa.free(indexables); |
| 7022 | // elements of this array can be `none`, indicating no length check. | 7022 | // elements of this array can be `none`, indicating no length check. |
| 7023 | const lens = try gpa.alloc(Zir.Inst.Ref, for_full.ast.inputs.len); | 7023 | const lens = try gpa.alloc([2]Zir.Inst.Ref, for_full.ast.inputs.len); |
| 7024 | defer gpa.free(lens); | 7024 | defer gpa.free(lens); |
| 7025 | 7025 | ||
| 7026 | // We will use a single zero-based counter no matter how many indexables there are. | 7026 | // We will use a single zero-based counter no matter how many indexables there are. |
| ... | @@ -7039,7 +7039,7 @@ fn forExpr( | ... | @@ -7039,7 +7039,7 @@ fn forExpr( |
| 7039 | 7039 | ||
| 7040 | { | 7040 | { |
| 7041 | var capture_token = for_full.payload_token; | 7041 | var capture_token = for_full.payload_token; |
| 7042 | for (for_full.ast.inputs, indexables, lens) |input, *indexable_ref, *len_ref| { | 7042 | for (for_full.ast.inputs, indexables, lens) |input, *indexable_ref, *len_refs| { |
| 7043 | const capture_is_ref = token_tags[capture_token] == .asterisk; | 7043 | const capture_is_ref = token_tags[capture_token] == .asterisk; |
| 7044 | const ident_tok = capture_token + @intFromBool(capture_is_ref); | 7044 | const ident_tok = capture_token + @intFromBool(capture_is_ref); |
| 7045 | const is_discard = mem.eql(u8, tree.tokenSlice(ident_tok), "_"); | 7045 | const is_discard = mem.eql(u8, tree.tokenSlice(ident_tok), "_"); |
| ... | @@ -7068,24 +7068,21 @@ fn forExpr( | ... | @@ -7068,24 +7068,21 @@ fn forExpr( |
| 7068 | try astgen.appendErrorTok(ident_tok, "discard of unbounded counter", .{}); | 7068 | try astgen.appendErrorTok(ident_tok, "discard of unbounded counter", .{}); |
| 7069 | } | 7069 | } |
| 7070 | 7070 | ||
| 7071 | const start_is_zero = nodeIsTriviallyZero(tree, start_node); | 7071 | if (end_val == .none) { |
| 7072 | const range_len = if (end_val == .none or start_is_zero) | 7072 | len_refs.* = .{ .none, .none }; |
| 7073 | end_val | 7073 | } else { |
| 7074 | else | 7074 | any_len_checks = true; |
| 7075 | try parent_gz.addPlNode(.sub, input, Zir.Inst.Bin{ | 7075 | len_refs.* = .{ start_val, end_val }; |
| 7076 | .lhs = end_val, | 7076 | } |
| 7077 | .rhs = start_val, | ||
| 7078 | }); | ||
| 7079 | 7077 | ||
| 7080 | any_len_checks = any_len_checks or range_len != .none; | 7078 | const start_is_zero = nodeIsTriviallyZero(tree, start_node); |
| 7081 | indexable_ref.* = if (start_is_zero) .none else start_val; | 7079 | indexable_ref.* = if (start_is_zero) .none else start_val; |
| 7082 | len_ref.* = range_len; | ||
| 7083 | } else { | 7080 | } else { |
| 7084 | const indexable = try expr(parent_gz, scope, .{ .rl = .none }, input); | 7081 | const indexable = try expr(parent_gz, scope, .{ .rl = .none }, input); |
| 7085 | 7082 | ||
| 7086 | any_len_checks = true; | 7083 | any_len_checks = true; |
| 7087 | indexable_ref.* = indexable; | 7084 | indexable_ref.* = indexable; |
| 7088 | len_ref.* = indexable; | 7085 | len_refs.* = .{ indexable, .none }; |
| 7089 | } | 7086 | } |
| 7090 | } | 7087 | } |
| 7091 | } | 7088 | } |
| ... | @@ -7097,12 +7094,13 @@ fn forExpr( | ... | @@ -7097,12 +7094,13 @@ fn forExpr( |
| 7097 | // We use a dedicated ZIR instruction to assert the lengths to assist with | 7094 | // We use a dedicated ZIR instruction to assert the lengths to assist with |
| 7098 | // nicer error reporting as well as fewer ZIR bytes emitted. | 7095 | // nicer error reporting as well as fewer ZIR bytes emitted. |
| 7099 | const len: Zir.Inst.Ref = len: { | 7096 | const len: Zir.Inst.Ref = len: { |
| 7100 | const lens_len: u32 = @intCast(lens.len); | 7097 | const all_lens = @as([*]Zir.Inst.Ref, @ptrCast(lens))[0 .. lens.len * 2]; |
| 7098 | const lens_len: u32 = @intCast(all_lens.len); | ||
| 7101 | try astgen.extra.ensureUnusedCapacity(gpa, @typeInfo(Zir.Inst.MultiOp).@"struct".fields.len + lens_len); | 7099 | try astgen.extra.ensureUnusedCapacity(gpa, @typeInfo(Zir.Inst.MultiOp).@"struct".fields.len + lens_len); |
| 7102 | const len = try parent_gz.addPlNode(.for_len, node, Zir.Inst.MultiOp{ | 7100 | const len = try parent_gz.addPlNode(.for_len, node, Zir.Inst.MultiOp{ |
| 7103 | .operands_len = lens_len, | 7101 | .operands_len = lens_len, |
| 7104 | }); | 7102 | }); |
| 7105 | appendRefsAssumeCapacity(astgen, lens); | 7103 | appendRefsAssumeCapacity(astgen, all_lens); |
| 7106 | break :len len; | 7104 | break :len len; |
| 7107 | }; | 7105 | }; |
| 7108 | 7106 |
lib/std/zig/Zir.zig+4-2
| ... | @@ -526,8 +526,10 @@ pub const Inst = struct { | ... | @@ -526,8 +526,10 @@ pub const Inst = struct { |
| 526 | /// Asserts that all the lengths provided match. Used to build a for loop. | 526 | /// Asserts that all the lengths provided match. Used to build a for loop. |
| 527 | /// Return value is the length as a usize. | 527 | /// Return value is the length as a usize. |
| 528 | /// Uses the `pl_node` field with payload `MultiOp`. | 528 | /// Uses the `pl_node` field with payload `MultiOp`. |
| 529 | /// There is exactly one item corresponding to each AST node inside the for | 529 | /// There are two items for each AST node inside the for loop condition. |
| 530 | /// loop condition. Any item may be `none`, indicating an unbounded range. | 530 | /// If both items in a pair are `.none`, then this node is an unbounded range. |
| 531 | /// If only the second item in a pair is `.none`, then the first is an indexable. | ||
| 532 | /// Otherwise, the node is a bounded range `a..b`, with the items being `a` and `b`. | ||
| 531 | /// Illegal behaviors: | 533 | /// Illegal behaviors: |
| 532 | /// * If all lengths are unbounded ranges (always a compile error). | 534 | /// * If all lengths are unbounded ranges (always a compile error). |
| 533 | /// * If any two lengths do not match each other. | 535 | /// * If any two lengths do not match each other. |
src/Sema.zig+20-23
| ... | @@ -4356,7 +4356,8 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. | ... | @@ -4356,7 +4356,8 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. |
| 4356 | const ip = &zcu.intern_pool; | 4356 | const ip = &zcu.intern_pool; |
| 4357 | const inst_data = sema.code.instructions.items(.data)[@intFromEnum(inst)].pl_node; | 4357 | const inst_data = sema.code.instructions.items(.data)[@intFromEnum(inst)].pl_node; |
| 4358 | const extra = sema.code.extraData(Zir.Inst.MultiOp, inst_data.payload_index); | 4358 | const extra = sema.code.extraData(Zir.Inst.MultiOp, inst_data.payload_index); |
| 4359 | const args = sema.code.refSlice(extra.end, extra.data.operands_len); | 4359 | const all_args = sema.code.refSlice(extra.end, extra.data.operands_len); |
| 4360 | const arg_pairs: []const [2]Zir.Inst.Ref = @as([*]const [2]Zir.Inst.Ref, @ptrCast(all_args))[0..@divExact(all_args.len, 2)]; | ||
| 4360 | const src = block.nodeOffset(inst_data.src_node); | 4361 | const src = block.nodeOffset(inst_data.src_node); |
| 4361 | 4362 | ||
| 4362 | var len: Air.Inst.Ref = .none; | 4363 | var len: Air.Inst.Ref = .none; |
| ... | @@ -4364,27 +4365,24 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. | ... | @@ -4364,27 +4365,24 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. |
| 4364 | var len_idx: u32 = undefined; | 4365 | var len_idx: u32 = undefined; |
| 4365 | var any_runtime = false; | 4366 | var any_runtime = false; |
| 4366 | 4367 | ||
| 4367 | const runtime_arg_lens = try gpa.alloc(Air.Inst.Ref, args.len); | 4368 | const runtime_arg_lens = try gpa.alloc(Air.Inst.Ref, arg_pairs.len); |
| 4368 | defer gpa.free(runtime_arg_lens); | 4369 | defer gpa.free(runtime_arg_lens); |
| 4369 | 4370 | ||
| 4370 | // First pass to look for comptime values. | 4371 | // First pass to look for comptime values. |
| 4371 | for (args, 0..) |zir_arg, i_usize| { | 4372 | for (arg_pairs, 0..) |zir_arg_pair, i_usize| { |
| 4372 | const i: u32 = @intCast(i_usize); | 4373 | const i: u32 = @intCast(i_usize); |
| 4373 | runtime_arg_lens[i] = .none; | 4374 | runtime_arg_lens[i] = .none; |
| 4374 | if (zir_arg == .none) continue; | 4375 | if (zir_arg_pair[0] == .none) continue; |
| 4375 | const object = try sema.resolveInst(zir_arg); | 4376 | |
| 4376 | const object_ty = sema.typeOf(object); | ||
| 4377 | // Each arg could be an indexable, or a range, in which case the length | ||
| 4378 | // is passed directly as an integer. | ||
| 4379 | const is_int = switch (object_ty.zigTypeTag(zcu)) { | ||
| 4380 | .int, .comptime_int => true, | ||
| 4381 | else => false, | ||
| 4382 | }; | ||
| 4383 | const arg_src = block.src(.{ .for_input = .{ | 4377 | const arg_src = block.src(.{ .for_input = .{ |
| 4384 | .for_node_offset = inst_data.src_node, | 4378 | .for_node_offset = inst_data.src_node, |
| 4385 | .input_index = i, | 4379 | .input_index = i, |
| 4386 | } }); | 4380 | } }); |
| 4387 | const arg_len_uncoerced = if (is_int) object else l: { | 4381 | |
| 4382 | const arg_len_uncoerced = if (zir_arg_pair[1] == .none) l: { | ||
| 4383 | // This argument is an indexable. | ||
| 4384 | const object = try sema.resolveInst(zir_arg_pair[0]); | ||
| 4385 | const object_ty = sema.typeOf(object); | ||
| 4388 | if (!object_ty.isIndexable(zcu)) { | 4386 | if (!object_ty.isIndexable(zcu)) { |
| 4389 | // Instead of using checkIndexable we customize this error. | 4387 | // Instead of using checkIndexable we customize this error. |
| 4390 | const msg = msg: { | 4388 | const msg = msg: { |
| ... | @@ -4401,8 +4399,12 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. | ... | @@ -4401,8 +4399,12 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. |
| 4401 | return sema.failWithOwnedErrorMsg(block, msg); | 4399 | return sema.failWithOwnedErrorMsg(block, msg); |
| 4402 | } | 4400 | } |
| 4403 | if (!object_ty.indexableHasLen(zcu)) continue; | 4401 | if (!object_ty.indexableHasLen(zcu)) continue; |
| 4404 | |||
| 4405 | break :l try sema.fieldVal(block, arg_src, object, try ip.getOrPutString(gpa, pt.tid, "len", .no_embedded_nulls), arg_src); | 4402 | break :l try sema.fieldVal(block, arg_src, object, try ip.getOrPutString(gpa, pt.tid, "len", .no_embedded_nulls), arg_src); |
| 4403 | } else l: { | ||
| 4404 | // This argument is a range. | ||
| 4405 | const range_start = try sema.resolveInst(zir_arg_pair[0]); | ||
| 4406 | const range_end = try sema.resolveInst(zir_arg_pair[1]); | ||
| 4407 | break :l try sema.analyzeArithmetic(block, .sub, range_end, range_start, arg_src, arg_src, arg_src, true); | ||
| 4406 | }; | 4408 | }; |
| 4407 | const arg_len = try sema.coerce(block, Type.usize, arg_len_uncoerced, arg_src); | 4409 | const arg_len = try sema.coerce(block, Type.usize, arg_len_uncoerced, arg_src); |
| 4408 | if (len == .none) { | 4410 | if (len == .none) { |
| ... | @@ -4444,17 +4446,12 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. | ... | @@ -4444,17 +4446,12 @@ fn zirForLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. |
| 4444 | const msg = msg: { | 4446 | const msg = msg: { |
| 4445 | const msg = try sema.errMsg(src, "unbounded for loop", .{}); | 4447 | const msg = try sema.errMsg(src, "unbounded for loop", .{}); |
| 4446 | errdefer msg.destroy(gpa); | 4448 | errdefer msg.destroy(gpa); |
| 4447 | for (args, 0..) |zir_arg, i_usize| { | 4449 | for (arg_pairs, 0..) |zir_arg_pair, i_usize| { |
| 4448 | const i: u32 = @intCast(i_usize); | 4450 | const i: u32 = @intCast(i_usize); |
| 4449 | if (zir_arg == .none) continue; | 4451 | if (zir_arg_pair[0] == .none) continue; |
| 4450 | const object = try sema.resolveInst(zir_arg); | 4452 | if (zir_arg_pair[1] != .none) continue; |
| 4453 | const object = try sema.resolveInst(zir_arg_pair[0]); | ||
| 4451 | const object_ty = sema.typeOf(object); | 4454 | const object_ty = sema.typeOf(object); |
| 4452 | // Each arg could be an indexable, or a range, in which case the length | ||
| 4453 | // is passed directly as an integer. | ||
| 4454 | switch (object_ty.zigTypeTag(zcu)) { | ||
| 4455 | .int, .comptime_int => continue, | ||
| 4456 | else => {}, | ||
| 4457 | } | ||
| 4458 | const arg_src = block.src(.{ .for_input = .{ | 4455 | const arg_src = block.src(.{ .for_input = .{ |
| 4459 | .for_node_offset = inst_data.src_node, | 4456 | .for_node_offset = inst_data.src_node, |
| 4460 | .input_index = i, | 4457 | .input_index = i, |
test/cases/compile_errors/for.zig+5-2
| ... | @@ -28,10 +28,11 @@ export fn d() void { | ... | @@ -28,10 +28,11 @@ export fn d() void { |
| 28 | _ = x3; | 28 | _ = x3; |
| 29 | } | 29 | } |
| 30 | } | 30 | } |
| 31 | export fn e() void { | ||
| 32 | for (123) |_| {} | ||
| 33 | } | ||
| 31 | 34 | ||
| 32 | // error | 35 | // error |
| 33 | // backend=stage2 | ||
| 34 | // target=native | ||
| 35 | // | 36 | // |
| 36 | // :2:5: error: non-matching for loop lengths | 37 | // :2:5: error: non-matching for loop lengths |
| 37 | // :2:11: note: length 10 here | 38 | // :2:11: note: length 10 here |
| ... | @@ -43,3 +44,5 @@ export fn d() void { | ... | @@ -43,3 +44,5 @@ export fn d() void { |
| 43 | // :25:5: error: unbounded for loop | 44 | // :25:5: error: unbounded for loop |
| 44 | // :25:10: note: type '[*]const u8' has no upper bound | 45 | // :25:10: note: type '[*]const u8' has no upper bound |
| 45 | // :25:18: note: type '[*]const u8' has no upper bound | 46 | // :25:18: note: type '[*]const u8' has no upper bound |
| 47 | // :32:10: error: type 'comptime_int' is not indexable and not a range | ||
| 48 | // :32:10: note: for loop operand must be a range, array, slice, tuple, or vector |