From f5ea78d65862e9046a996823589fd88827b00524 Mon Sep 17 00:00:00 2001 From: Matthew Lugg Date: Sun, 8 Feb 2026 13:10:21 +0000 Subject: [PATCH] compiler: error set bugfixes --- src/Sema.zig | 123 +++++++++++++++++++++++++++++++++++++-------------- src/Type.zig | 63 +++++++++++++++----------- 2 files changed, 127 insertions(+), 59 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 0504bb7e5e..b0271ffde0 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -15629,10 +15629,10 @@ fn zirCmpEq( // comparing null with optionals if (lhs_ty_tag == .null and (rhs_ty_tag == .optional or rhs_ty.isCPtr(zcu))) { - return sema.analyzeIsNull(block, rhs, op == .neq); + return sema.analyzeIsNull(block, src, rhs, op == .neq); } if (rhs_ty_tag == .null and (lhs_ty_tag == .optional or lhs_ty.isCPtr(zcu))) { - return sema.analyzeIsNull(block, lhs, op == .neq); + return sema.analyzeIsNull(block, src, lhs, op == .neq); } if (lhs_ty_tag == .null or rhs_ty_tag == .null) { @@ -17375,7 +17375,7 @@ fn zirIsNonNull( const src = block.nodeOffset(inst_data.src_node); const operand = sema.resolveInst(inst_data.operand); try sema.checkNullableType(block, src, sema.typeOf(operand)); - return sema.analyzeIsNull(block, operand, true); + return sema.analyzeIsNull(block, src, operand, true); } fn zirIsNonNullPtr( @@ -17394,15 +17394,19 @@ fn zirIsNonNullPtr( const ptr_ty = sema.typeOf(ptr); assert(ptr_ty.zigTypeTag(zcu) == .pointer); const nullable_ty = ptr_ty.childType(zcu); + try sema.checkNullableType(block, src, nullable_ty); + + if (try sema.resolveIsNullFromType(block, src, nullable_ty)) |is_null| { + return .fromValue(.makeBool(!is_null)); + } + if (sema.resolveValue(ptr)) |ptr_val| { if (try sema.pointerDeref(block, src, ptr_val, ptr_ty)) |nullable_val| { - return sema.analyzeIsNull(block, .fromValue(nullable_val), true); + return sema.analyzeIsNull(block, src, .fromValue(nullable_val), true); } } - if (nullable_ty.isNullFromType(zcu)) |is_null| { - return if (is_null) .bool_false else .bool_true; - } + return block.addUnOp(.is_non_null_ptr, ptr); } @@ -30147,25 +30151,25 @@ fn analyzeSliceLen( fn analyzeIsNull( sema: *Sema, block: *Block, + src: LazySrcLoc, operand: Air.Inst.Ref, invert_logic: bool, ) CompileError!Air.Inst.Ref { const pt = sema.pt; const zcu = pt.zcu; - const result_ty: Type = .bool; - if (sema.resolveValue(operand)) |opt_val| { - if (opt_val.isUndef(zcu)) { - return pt.undefRef(result_ty); - } - const is_null = opt_val.isNull(zcu); - const bool_value = if (invert_logic) !is_null else is_null; - return if (bool_value) .bool_true else .bool_false; + + if (try sema.resolveIsNullFromType(block, src, sema.typeOf(operand))) |is_null| { + return .fromValue(.makeBool(is_null != invert_logic)); // XOR } - if (sema.typeOf(operand).isNullFromType(zcu)) |is_null| { - const result = is_null != invert_logic; - return if (result) .bool_true else .bool_false; + if (sema.resolveValue(operand)) |opt_val| { + if (opt_val.isUndef(zcu)) { + return pt.undefRef(.bool); + } + const is_null = opt_val.isNull(zcu); + return .fromValue(.makeBool(is_null != invert_logic)); // XOR } + const air_tag: Air.Inst.Tag = if (invert_logic) .is_non_null else .is_null; return block.addUnOp(air_tag, operand); } @@ -30218,6 +30222,35 @@ fn resolveIsNonErrVal( return null; } +fn resolveIsNullFromType( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + ty: Type, +) CompileError!?bool { + const zcu = sema.pt.zcu; + return switch (ty.zigTypeTag(zcu)) { + else => false, + .null => true, + .pointer => switch (ty.ptrSize(zcu)) { + .c => null, + else => false, + }, + .optional => { + const payload_ty = ty.optionalChild(zcu); + if (payload_ty.classify(zcu) == .no_possible_value) { + return true; // e.g. `?noreturn` + } + if (payload_ty.zigTypeTag(zcu) == .error_set and + try sema.resolveErrSetIsEmpty(block, src, payload_ty)) + { + return true; // e.g. `?error{}` + } + return null; + }, + }; +} + fn resolveIsNonErrFromType( sema: *Sema, block: *Block, @@ -30226,7 +30259,6 @@ fn resolveIsNonErrFromType( ) CompileError!?Value { const pt = sema.pt; const zcu = pt.zcu; - const ip = &zcu.intern_pool; const ot = operand_ty.zigTypeTag(zcu); if (ot != .error_set and ot != .error_union) return .true; if (ot == .error_set) return .false; @@ -30236,29 +30268,54 @@ fn resolveIsNonErrFromType( if (payload_ty.classify(zcu) == .no_possible_value) { return .false; } + if (try sema.resolveErrSetIsEmpty(block, src, operand_ty.errorUnionSet(zcu))) { + return .true; + } + return null; +} - // exception if the error union error set is known to be empty, - // we allow the comparison but always make it comptime-known. - return err_set: switch (ip.errorUnionSet(operand_ty.toIntern())) { - .anyerror_type => null, +/// Returns `true` iff the error set type `orig_err_set_ty` contains no errors. +/// +/// This is used to give comptime answers for whether `error{}!T` is an error or a payload, as well +/// as whether `?error{}` is null. The type `error{}` cannot be NPV, as it has runtime bits, but the +/// only value of that type which can exist is `undefined`; semantically it has no "legal" value. +/// TODO: this runs into some unsolved language design questions about such types. Performing a +/// coercion from `@as(E, undefined)` to `E!T` needs to semantically result in an `undefined` error +/// union if our implementation is to be legal, and likewise for coercing `@as(E, undefined)` to +/// `?E` (for an error set `E`) because our implementation uses the zero error value at runtime to +/// represent `null`. The unsolved problem is the exact rules for `undefined` propagation through +/// these types: for instance, what if `@as(u32, undfined)` is coerced to `?u32`? What about error +/// union *payloads*, i.e. `@as(u32, undefined)` to `E!u32`? That one is analagous to the optional +/// example in some ways, but right now I believe there is code which relies on that coercion giving +/// a well-defined error union with an `undefined` payload. +/// Relevant issues/discussions: +/// * https://github.com/ziglang/zig/issues/1831 +/// * https://github.com/ziglang/zig/issues/6762 +/// * https://github.com/ziglang/zig/issues/1831#issuecomment-722129239 +fn resolveErrSetIsEmpty( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + orig_err_set_ty: Type, +) CompileError!bool { + const ip = &sema.pt.zcu.intern_pool; + err_set: switch (orig_err_set_ty.toIntern()) { + .anyerror_type => return false, .adhoc_inferred_error_set_type => { // This is *our* error set; that is, we're currently analyzing the function // which owns it. Trying to resolve it now would cause a dependency loop. // Instead, accept that we don't know. - return null; + return false; }, - else => |set_ty| switch (ip.indexToKey(set_ty)) { - .error_set_type => |error_set_type| switch (error_set_type.names.len) { - 0 => .true, - else => null, - }, + else => |err_set_ty| switch (ip.indexToKey(err_set_ty)) { + .error_set_type => |es| return es.names.len == 0, .inferred_error_set_type => |func_index| { if (sema.fn_ret_ty_ies) |ies| { if (ies.func == func_index) { // This is *our* error set; that is, we're currently analyzing the function // which owns it. Trying to resolve it now would cause a dependency loop. // Instead, accept that we don't know. - return null; + return false; } } try sema.ensureFuncIesResolved(block, src, func_index); @@ -30266,7 +30323,7 @@ fn resolveIsNonErrFromType( }, else => unreachable, }, - }; + } } fn analyzeIsNonErr( @@ -30732,7 +30789,7 @@ fn analyzeSlice( if (block.wantSafety()) { // requirement: slicing C ptr is non-null if (ptr_ptr_child_ty.isCPtr(zcu)) { - const is_non_null = try sema.analyzeIsNull(block, ptr, true); + const is_non_null = try block.addUnOp(.is_non_null, ptr); try sema.addSafetyCheck(block, src, is_non_null, .unwrap_null); } @@ -30792,7 +30849,7 @@ fn analyzeSlice( if (block.wantSafety()) { // requirement: slicing C ptr is non-null if (ptr_ptr_child_ty.isCPtr(zcu)) { - const is_non_null = try sema.analyzeIsNull(block, ptr, true); + const is_non_null = try block.addUnOp(.is_non_null, ptr); try sema.addSafetyCheck(block, src, is_non_null, .unwrap_null); } diff --git a/src/Type.zig b/src/Type.zig index 81ee1e560e..3eb14f3b53 100644 --- a/src/Type.zig +++ b/src/Type.zig @@ -661,15 +661,28 @@ pub fn toValue(self: Type) Value { return .fromInterned(self.toIntern()); } -/// true if and only if the type takes up space in memory at runtime. -/// There are two reasons a type will return false: -/// * the type is a comptime-only type. For example, the type `type` itself. -/// - note, however, that a struct can have mixed fields and only the non-comptime-only -/// fields will count towards the ABI size. For example, `struct {T: type, x: i32}` -/// hasRuntimeBits()=true and abiSize()=4 -/// * the type has only one possible value, making its ABI size 0. -/// - an enum with an explicit tag type has the ABI size of the integer tag type, -/// making it one-possible-value only if the integer tag type has 0 bits. +/// Returns `true` if and only if the type takes up space in memory at runtime. This is also exactly +/// whether or not the backend/linker needs to be sent values of this type to emit to the binary. +/// +/// Types without runtime bits have an ABI size of 0; all other types have a non-zero ABI size. All +/// types, regardless of whether they have runtime bits, have a non-zero ABI alignment. +/// +/// Comptime-only types may still have runtime bits. For instance, `struct { a: u32, b: type }` is a +/// comptime-only type, but it nonetheless has runtime bits and a runtime memory layout (where the +/// field `b: type` is omitted). This is because a user may take a pointer to the field `a`, which +/// must then be valid to use at runtime. +/// +/// This function is a trivial wrapper around `classify`: +/// +/// * Types with one possible value, such as `void`, or no possible value, such as `noreturn`, do +/// not have runtime bits and have an ABI size of 0 because they simply contain no state. +/// +/// * Types which are fully comptime, such as `type` and `comptime_int`, do not have runtime bits +/// because they contain only comptime state. (This compiler implementation also currently makes +/// types like `struct { x: comptime_int }` fully comptime, but that could change in the future if +/// we start inserting hidden safety fields into them.) +/// +/// * All other types contain some runtime state, so have runtime bits and a non-zero ABI size. pub fn hasRuntimeBits(ty: Type, zcu: *const Zcu) bool { return switch (ty.classify(zcu)) { .no_possible_value, .one_possible_value, .fully_comptime => false, @@ -1576,6 +1589,11 @@ pub fn errorUnionSet(ty: Type, zcu: *const Zcu) Type { } /// Returns false for unresolved inferred error sets. +/// +/// TODO: this function will behave incorrectly under incremental compilation, because in that case +/// it may see an outdated resolved error set. This function must be either deleted, or its contract +/// changed to require the caller to resolve the error set beforehand. If you must introduce new +/// call sites, please make sure the error set in question is definitely resolved first! pub fn errorSetIsEmpty(ty: Type, zcu: *const Zcu) bool { const ip = &zcu.intern_pool; return switch (ty.toIntern()) { @@ -1594,6 +1612,11 @@ pub fn errorSetIsEmpty(ty: Type, zcu: *const Zcu) bool { /// Returns true if it is an error set that includes anyerror, false otherwise. /// Note that the result may be a false negative if the type did not get error set /// resolution prior to this call. +/// +/// TODO: this function will behave incorrectly under incremental compilation, because in that case +/// it may see an outdated resolved error set. This function must be either deleted, or its contract +/// changed to require the caller to resolve the error set beforehand. If you must introduce new +/// call sites, please make sure the error set in question is definitely resolved first! pub fn isAnyError(ty: Type, zcu: *const Zcu) bool { const ip = &zcu.intern_pool; return switch (ty.toIntern()) { @@ -1616,6 +1639,11 @@ pub fn isError(ty: Type, zcu: *const Zcu) bool { /// Returns whether ty, which must be an error set, includes an error `name`. /// Might return a false negative if `ty` is an inferred error set and not fully /// resolved yet. +/// +/// TODO: this function will behave incorrectly under incremental compilation, because in that case +/// it may see an outdated resolved error set. This function must be either deleted, or its contract +/// changed to require the caller to resolve the error set beforehand. If you must introduce new +/// call sites, please make sure the error set in question is definitely resolved first! pub fn errorSetHasField( ty: Type, name: InternPool.NullTerminatedString, @@ -2939,23 +2967,6 @@ pub fn containerTypeName(ty: Type, ip: *const InternPool) InternPool.NullTermina }; } -/// Returns `true` if a value of this type is always `null`. -/// Returns `false` if a value of this type is never `null`. -/// Returns `null` otherwise. -pub fn isNullFromType(ty: Type, zcu: *const Zcu) ?bool { - if (ty.zigTypeTag(zcu) != .optional and !ty.isCPtr(zcu)) return false; - const payload_ty = ty.optionalChild(zcu); - if (payload_ty.classify(zcu) == .no_possible_value) return true; // `?noreturn` etc - - // Although it has runtime bits, `?error{}` is always null. MLUGG TODO: think for a bit... - switch (zcu.intern_pool.indexToKey(payload_ty.toIntern())) { - .error_set_type => |error_set| if (error_set.names.len == 0) return true, - else => {}, - } - - return null; -} - pub const UnpackableReason = union(enum) { comptime_only, pointer,