From 05762ca02ff97e7abfe1b52c090663dbf99bd4fc Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Thu, 7 Aug 2025 13:02:01 +0200 Subject: [PATCH] address most comments --- src/InternPool.zig | 8 +- src/Sema.zig | 72 ++++++++++-------- src/Sema/arith.zig | 75 ++++--------------- src/Zcu/PerThread.zig | 2 +- .../shift_by_larger_than_usize.zig | 1 - .../shlExact_shifts_out_1_bits.zig | 2 +- 6 files changed, 61 insertions(+), 99 deletions(-) diff --git a/src/InternPool.zig b/src/InternPool.zig index 65b7b52180..0aa664d611 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -2036,6 +2036,8 @@ pub const Key = union(enum) { /// Each element/field stored as an `Index`. /// In the case of sentinel-terminated arrays, the sentinel value *is* stored, /// so the slice length will be one more than the type's array length. + /// There must be at least one element which is not `undefined`. If all elements are + /// undefined, instead create an undefined value of the aggregate type. aggregate: Aggregate, /// An instance of a union. un: Union, @@ -8408,7 +8410,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, tid: Zcu.PerThread.Id, key: Key) All if (!ip.isUndef(elem)) any_defined = true; assert(ip.typeOf(elem) == child); } - assert(any_defined); + assert(any_defined); // aggregate fields must not be all undefined }, .struct_type => { var any_defined = false; @@ -8416,7 +8418,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, tid: Zcu.PerThread.Id, key: Key) All if (!ip.isUndef(elem)) any_defined = true; assert(ip.typeOf(elem) == field_ty); } - assert(any_defined); + assert(any_defined); // aggregate fields must not be all undefined }, .tuple_type => |tuple_type| { var any_defined = false; @@ -8424,7 +8426,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, tid: Zcu.PerThread.Id, key: Key) All if (!ip.isUndef(elem)) any_defined = true; assert(ip.typeOf(elem) == ty); } - assert(any_defined); + assert(any_defined); // aggregate fields must not be all undefined }, else => unreachable, }; diff --git a/src/Sema.zig b/src/Sema.zig index d653e39bbf..26400b5a91 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -13637,6 +13637,8 @@ fn zirShl( const scalar_ty = lhs_ty.scalarType(zcu); const scalar_rhs_ty = rhs_ty.scalarType(zcu); + // AstGen currently forces the rhs of `<<` to coerce to the correct type before the `.shl` instruction, so + // we already know `scalar_rhs_ty` is valid for `.shl` -- we only need to validate for `.shl_sat`. if (air_tag == .shl_sat) _ = try sema.checkIntType(block, rhs_src, scalar_rhs_ty); const maybe_lhs_val = try sema.resolveValueResolveLazy(lhs); @@ -13645,25 +13647,29 @@ fn zirShl( const runtime_src = rs: { if (maybe_rhs_val) |rhs_val| { if (maybe_lhs_val) |lhs_val| { - return Air.internedToRef((try arith.shl(sema, block, lhs_ty, lhs_val, rhs_val, lhs_src, rhs_src, switch (air_tag) { + return .fromValue(try arith.shl(sema, block, lhs_ty, lhs_val, rhs_val, src, lhs_src, rhs_src, switch (air_tag) { .shl => .shl, .shl_sat => .shl_sat, .shl_exact => .shl_exact, else => unreachable, - })).toIntern()); + })); } if (rhs_val.isUndef(zcu)) switch (air_tag) { .shl_sat => return pt.undefRef(lhs_ty), .shl, .shl_exact => return sema.failWithUseOfUndef(block, rhs_src, null), else => unreachable, }; - const bits_val = try pt.intValue(.comptime_int, scalar_ty.intInfo(zcu).bits); + const bits = scalar_ty.intInfo(zcu).bits; switch (rhs_ty.zigTypeTag(zcu)) { .int, .comptime_int => { switch (try rhs_val.orderAgainstZeroSema(pt)) { .gt => { - if (air_tag != .shl_sat and try rhs_val.compareHeteroSema(.gte, bits_val, pt)) { - return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_val, rhs_src, null); + if (air_tag != .shl_sat) { + var rhs_space: Value.BigIntSpace = undefined; + const rhs_bigint = try rhs_val.toBigIntSema(&rhs_space, pt); + if (rhs_bigint.orderAgainstScalar(bits) != .lt) { + return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_val, rhs_src, null); + } } }, .eq => return lhs, @@ -13672,8 +13678,7 @@ fn zirShl( }, .vector => { var any_positive: bool = false; - var elem_idx: usize = 0; - while (elem_idx < rhs_ty.vectorLen(zcu)) : (elem_idx += 1) { + for (0..rhs_ty.vectorLen(zcu)) |elem_idx| { const rhs_elem = try rhs_val.elemValue(pt, elem_idx); if (rhs_elem.isUndef(zcu)) switch (air_tag) { .shl_sat => continue, @@ -13682,8 +13687,12 @@ fn zirShl( }; switch (try rhs_elem.orderAgainstZeroSema(pt)) { .gt => { - if (air_tag != .shl_sat and try rhs_elem.compareHeteroSema(.gte, bits_val, pt)) { - return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_elem, rhs_src, elem_idx); + if (air_tag != .shl_sat) { + var rhs_elem_space: Value.BigIntSpace = undefined; + const rhs_elem_bigint = try rhs_elem.toBigIntSema(&rhs_elem_space, pt); + if (rhs_elem_bigint.orderAgainstScalar(bits) != .lt) { + return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_elem, rhs_src, elem_idx); + } } any_positive = true; }, @@ -13713,29 +13722,29 @@ fn zirShl( } break :rs rhs_src; }; - const rt_rhs = switch (air_tag) { + const rt_rhs: Air.Inst.Ref = switch (air_tag) { else => unreachable, .shl, .shl_exact => rhs, // The backend can handle a large runtime rhs better than we can, but // we can limit a large comptime rhs better here. This also has the // necessary side effect of preventing rhs from being a `comptime_int`. - .shl_sat => if (maybe_rhs_val) |rhs_val| Air.internedToRef(rt_rhs: { + .shl_sat => if (maybe_rhs_val) |rhs_val| .fromValue(rt_rhs: { const bit_count = scalar_ty.intInfo(zcu).bits; const rt_rhs_scalar_ty = try pt.smallestUnsignedInt(bit_count); - if (!rhs_ty.isVector(zcu)) break :rt_rhs (try pt.intValue( + if (!rhs_ty.isVector(zcu)) break :rt_rhs try pt.intValue( rt_rhs_scalar_ty, @min(try rhs_val.getUnsignedIntSema(pt) orelse bit_count, bit_count), - )).toIntern(); + ); const rhs_len = rhs_ty.vectorLen(zcu); const rhs_elems = try sema.arena.alloc(InternPool.Index, rhs_len); for (rhs_elems, 0..) |*rhs_elem, i| rhs_elem.* = (try pt.intValue( rt_rhs_scalar_ty, @min(try (try rhs_val.elemValue(pt, i)).getUnsignedIntSema(pt) orelse bit_count, bit_count), )).toIntern(); - break :rt_rhs (try pt.aggregateValue(try pt.vectorType(.{ + break :rt_rhs try pt.aggregateValue(try pt.vectorType(.{ .len = rhs_len, .child = rt_rhs_scalar_ty.toIntern(), - }), rhs_elems)).toIntern(); + }), rhs_elems); }) else rhs, }; @@ -13760,7 +13769,7 @@ fn zirShl( const op_ov = try block.addInst(.{ .tag = .shl_with_overflow, .data = .{ .ty_pl = .{ - .ty = Air.internedToRef(op_ov_tuple_ty.toIntern()), + .ty = .fromIntern(op_ov_tuple_ty.toIntern()), .payload = try sema.addExtra(Air.Bin{ .lhs = lhs, .rhs = rhs, @@ -13820,21 +13829,23 @@ fn zirShr( const runtime_src = rs: { if (maybe_rhs_val) |rhs_val| { if (maybe_lhs_val) |lhs_val| { - return Air.internedToRef((try arith.shr(sema, block, lhs_ty, rhs_ty, lhs_val, rhs_val, src, lhs_src, rhs_src, switch (air_tag) { + return .fromValue(try arith.shr(sema, block, lhs_ty, rhs_ty, lhs_val, rhs_val, src, lhs_src, rhs_src, switch (air_tag) { .shr => .shr, .shr_exact => .shr_exact, else => unreachable, - })).toIntern()); + })); } if (rhs_val.isUndef(zcu)) { return sema.failWithUseOfUndef(block, rhs_src, null); } - const bits_val = try pt.intValue(.comptime_int, scalar_ty.intInfo(zcu).bits); + const bits = scalar_ty.intInfo(zcu).bits; switch (rhs_ty.zigTypeTag(zcu)) { .int, .comptime_int => { switch (try rhs_val.orderAgainstZeroSema(pt)) { .gt => { - if (try rhs_val.compareHeteroSema(.gte, bits_val, pt)) { + var rhs_space: Value.BigIntSpace = undefined; + const rhs_bigint = try rhs_val.toBigIntSema(&rhs_space, pt); + if (rhs_bigint.orderAgainstScalar(bits) != .lt) { return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_val, rhs_src, null); } }, @@ -13844,16 +13855,17 @@ fn zirShr( }, .vector => { var any_positive: bool = false; - var elem_idx: usize = 0; - while (elem_idx < rhs_ty.vectorLen(zcu)) : (elem_idx += 1) { + for (0..rhs_ty.vectorLen(zcu)) |elem_idx| { const rhs_elem = try rhs_val.elemValue(pt, elem_idx); if (rhs_elem.isUndef(zcu)) { return sema.failWithUseOfUndef(block, rhs_src, elem_idx); } switch (try rhs_elem.orderAgainstZeroSema(pt)) { .gt => { - if (try rhs_elem.compareHeteroSema(.gte, bits_val, pt)) { - return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_val, rhs_src, elem_idx); + var rhs_elem_space: Value.BigIntSpace = undefined; + const rhs_elem_bigint = try rhs_elem.toBigIntSema(&rhs_elem_space, pt); + if (rhs_elem_bigint.orderAgainstScalar(bits) != .lt) { + return sema.failWithTooLargeShiftAmount(block, lhs_ty, rhs_elem, rhs_src, elem_idx); } any_positive = true; }, @@ -22718,7 +22730,6 @@ fn zirByteSwap(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai const pt = sema.pt; const zcu = pt.zcu; const inst_data = sema.code.instructions.items(.data)[@intFromEnum(inst)].un_node; - const src = block.nodeOffset(inst_data.src_node); const operand_src = block.builtinCallArgSrc(inst_data.src_node, 0); const operand = try sema.resolveInst(inst_data.operand); const operand_ty = sema.typeOf(operand); @@ -22733,30 +22744,27 @@ fn zirByteSwap(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai ); } if (try sema.typeHasOnePossibleValue(operand_ty)) |val| { - return Air.internedToRef(val.toIntern()); + return .fromValue(val); } if (try sema.resolveValue(operand)) |operand_val| { - return Air.internedToRef((try arith.byteSwap(sema, operand_val, operand_ty)).toIntern()); + return .fromValue(try arith.byteSwap(sema, operand_val, operand_ty)); } - try sema.requireRuntimeBlock(block, src, operand_src); return block.addTyOp(.byte_swap, operand_ty, operand); } fn zirBitReverse(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Inst.Ref { const inst_data = sema.code.instructions.items(.data)[@intFromEnum(inst)].un_node; - const src = block.nodeOffset(inst_data.src_node); const operand_src = block.builtinCallArgSrc(inst_data.src_node, 0); const operand = try sema.resolveInst(inst_data.operand); const operand_ty = sema.typeOf(operand); _ = try sema.checkIntOrVector(block, operand, operand_src); if (try sema.typeHasOnePossibleValue(operand_ty)) |val| { - return Air.internedToRef(val.toIntern()); + return .fromValue(val); } if (try sema.resolveValue(operand)) |operand_val| { - return Air.internedToRef((try arith.bitReverse(sema, operand_val, operand_ty)).toIntern()); + return .fromValue(try arith.bitReverse(sema, operand_val, operand_ty)); } - try sema.requireRuntimeBlock(block, src, operand_src); return block.addTyOp(.bit_reverse, operand_ty, operand); } diff --git a/src/Sema/arith.zig b/src/Sema/arith.zig index a2d931295d..8a5d18e315 100644 --- a/src/Sema/arith.zig +++ b/src/Sema/arith.zig @@ -319,16 +319,7 @@ pub fn add( const rhs_elem = try rhs_val.elemValue(pt, elem_idx); result_elem.* = (try addScalar(sema, block, elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, is_int, elem_idx)).toIntern(); } - - if (is_int) { - const result_val = try pt.intern(.{ .aggregate = .{ - .ty = ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } }); - return .fromInterned(result_val); - } else { - return pt.aggregateValue(ty, elem_vals); - } + return pt.aggregateValue(ty, elem_vals); }, else => unreachable, } @@ -482,16 +473,7 @@ pub fn sub( const rhs_elem = try rhs_val.elemValue(pt, elem_idx); result_elem.* = (try subScalar(sema, block, elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, is_int, elem_idx)).toIntern(); } - - if (is_int) { - const result_val = try pt.intern(.{ .aggregate = .{ - .ty = ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } }); - return .fromInterned(result_val); - } else { - return pt.aggregateValue(ty, elem_vals); - } + return pt.aggregateValue(ty, elem_vals); }, else => unreachable, } @@ -654,16 +636,7 @@ pub fn mul( const rhs_elem = try rhs_val.elemValue(pt, elem_idx); result_elem.* = (try mulScalar(sema, block, elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, is_int, elem_idx)).toIntern(); } - - if (is_int) { - const result_val = try pt.intern(.{ .aggregate = .{ - .ty = ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } }); - return .fromInterned(result_val); - } else { - return pt.aggregateValue(ty, elem_vals); - } + return pt.aggregateValue(ty, elem_vals); }, else => unreachable, } @@ -829,16 +802,7 @@ pub fn div( const rhs_elem = try rhs_val.elemValue(pt, elem_idx); result_elem.* = (try divScalar(sema, block, elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, op, is_int, elem_idx)).toIntern(); } - - if (is_int) { - const result_val = try pt.intern(.{ .aggregate = .{ - .ty = ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } }); - return .fromInterned(result_val); - } else { - return pt.aggregateValue(ty, elem_vals); - } + return pt.aggregateValue(ty, elem_vals); }, else => unreachable, } @@ -998,13 +962,14 @@ pub const ShlOp = enum { shl, shl_sat, shl_exact }; /// Applies the `<<` operator to comptime-known values. /// `lhs_ty` is an int, comptime_int, or vector thereof. -/// If it is a vector, he type of `rhs` has to also be a vector of the same length. +/// If it is a vector, the type of `rhs` has to also be a vector of the same length. pub fn shl( sema: *Sema, block: *Block, lhs_ty: Type, lhs_val: Value, rhs_val: Value, + src: LazySrcLoc, lhs_src: LazySrcLoc, rhs_src: LazySrcLoc, op: ShlOp, @@ -1012,7 +977,7 @@ pub fn shl( const pt = sema.pt; const zcu = pt.zcu; switch (lhs_ty.zigTypeTag(zcu)) { - .int, .comptime_int => return shlScalar(sema, block, lhs_ty, lhs_val, rhs_val, lhs_src, rhs_src, op, null), + .int, .comptime_int => return shlScalar(sema, block, lhs_ty, lhs_val, rhs_val, src, lhs_src, rhs_src, op, null), .vector => { const lhs_elem_ty = lhs_ty.childType(zcu); const len = lhs_ty.vectorLen(zcu); @@ -1021,22 +986,15 @@ pub fn shl( for (elem_vals, 0..) |*result_elem, elem_idx| { const lhs_elem = try lhs_val.elemValue(pt, elem_idx); const rhs_elem = try rhs_val.elemValue(pt, elem_idx); - result_elem.* = (try shlScalar(sema, block, lhs_elem_ty, lhs_elem, rhs_elem, lhs_src, rhs_src, op, elem_idx)).toIntern(); - } - if (op == .shl_sat) { - return pt.aggregateValue(lhs_ty, elem_vals); - } else { - return .fromInterned(try pt.intern(.{ .aggregate = .{ - .ty = lhs_ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } })); + result_elem.* = (try shlScalar(sema, block, lhs_elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, op, elem_idx)).toIntern(); } + return pt.aggregateValue(lhs_ty, elem_vals); }, else => unreachable, } } /// `lhs_ty` is an int, comptime_int, or vector thereof. -/// If it is a vector, he type of `rhs` has to also be a vector of the same length. +/// If it is a vector, the type of `rhs` has to also be a vector of the same length. pub fn shlWithOverflow( sema: *Sema, block: *Block, @@ -1084,6 +1042,7 @@ fn shlScalar( lhs_ty: Type, lhs_val: Value, rhs_val: Value, + src: LazySrcLoc, lhs_src: LazySrcLoc, rhs_src: LazySrcLoc, op: ShlOp, @@ -1114,7 +1073,7 @@ fn shlScalar( .shl_exact => { const shifted = try intShlWithOverflow(sema, block, lhs_ty, lhs_val, rhs_val, rhs_src, false, vec_idx); if (shifted.overflow) { - return sema.failWithIntegerOverflow(block, lhs_src, lhs_ty, shifted.val, vec_idx); + return sema.failWithIntegerOverflow(block, src, lhs_ty, shifted.val, vec_idx); } return shifted.val; }, @@ -1164,7 +1123,7 @@ pub const ShrOp = enum { shr, shr_exact }; /// Applies the `>>` operator to comptime-known values. /// `lhs_ty` is an int, comptime_int, or vector thereof. -/// If it is a vector, he type of `rhs` has to also be a vector of the same length. +/// If it is a vector, the type of `rhs` has to also be a vector of the same length. pub fn shr( sema: *Sema, block: *Block, @@ -1193,13 +1152,7 @@ pub fn shr( const rhs_elem = try rhs_val.elemValue(pt, elem_idx); result_elem.* = (try shrScalar(sema, block, lhs_elem_ty, rhs_elem_ty, lhs_elem, rhs_elem, src, lhs_src, rhs_src, op, elem_idx)).toIntern(); } - switch (op) { - .shr => return pt.aggregateValue(lhs_ty, elem_vals), - .shr_exact => return .fromInterned(try pt.intern(.{ .aggregate = .{ - .ty = lhs_ty.toIntern(), - .storage = .{ .elems = elem_vals }, - } })), - } + return pt.aggregateValue(lhs_ty, elem_vals); }, else => unreachable, } diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 0325c915ed..6a86bb76a1 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -3675,7 +3675,7 @@ pub fn unionValue(pt: Zcu.PerThread, union_ty: Type, tag: Value, val: Value) All pub fn aggregateValue(pt: Zcu.PerThread, ty: Type, elems: []const InternPool.Index) Allocator.Error!Value { for (elems) |elem| { if (!Value.fromInterned(elem).isUndef(pt.zcu)) break; - } else { // all-undef + } else if (elems.len > 0) { // all-undef return pt.undefValue(ty); } return .fromInterned(try pt.intern(.{ .aggregate = .{ diff --git a/test/cases/compile_errors/shift_by_larger_than_usize.zig b/test/cases/compile_errors/shift_by_larger_than_usize.zig index e956f5a767..1398bb859f 100644 --- a/test/cases/compile_errors/shift_by_larger_than_usize.zig +++ b/test/cases/compile_errors/shift_by_larger_than_usize.zig @@ -4,7 +4,6 @@ export fn f() usize { } // error -// backend=stage2,llvm // target=x86_64-linux // // :2:30: error: this implementation only supports comptime shift amounts of up to 2^64 - 1 bits diff --git a/test/cases/compile_errors/shlExact_shifts_out_1_bits.zig b/test/cases/compile_errors/shlExact_shifts_out_1_bits.zig index 35105c3896..aa9db75482 100644 --- a/test/cases/compile_errors/shlExact_shifts_out_1_bits.zig +++ b/test/cases/compile_errors/shlExact_shifts_out_1_bits.zig @@ -7,4 +7,4 @@ comptime { // backend=stage2 // target=native // -// :2:25: error: overflow of integer type 'u8' with value '340' +// :2:15: error: overflow of integer type 'u8' with value '340'