Commit 8bcb578507

mlugg <mlugg@mlugg.co.uk>
2025-01-20 14:34:47
Sema: fix `is_non_null_ptr` handling for runtime-known pointers
We can still often determine a comptime result based on the type, even if the pointer is runtime-known. Also, we previously used load -> is non null instead of AIR `is_non_null_ptr` if the pointer is comptime-known, but that's a bad heuristic. Instead, we should check for the pointer to be comptime-known, *and* for the load to be comptime-known, and only in that case should we call `Sema.analyzeIsNonNull`. Resolves: #22556
1 parent b9198b7
Changed files (4)
src/Sema.zig
@@ -17477,10 +17477,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, src, rhs, op == .neq);
+        return sema.analyzeIsNull(block, rhs, op == .neq);
     }
     if (rhs_ty_tag == .null and (lhs_ty_tag == .optional or lhs_ty.isCPtr(zcu))) {
-        return sema.analyzeIsNull(block, src, lhs, op == .neq);
+        return sema.analyzeIsNull(block, lhs, op == .neq);
     }
 
     if (lhs_ty_tag == .null or rhs_ty_tag == .null) {
@@ -19326,7 +19326,7 @@ fn zirIsNonNull(
     const src = block.nodeOffset(inst_data.src_node);
     const operand = try sema.resolveInst(inst_data.operand);
     try sema.checkNullableType(block, src, sema.typeOf(operand));
-    return sema.analyzeIsNull(block, src, operand, true);
+    return sema.analyzeIsNull(block, operand, true);
 }
 
 fn zirIsNonNullPtr(
@@ -19342,12 +19342,17 @@ fn zirIsNonNullPtr(
     const inst_data = sema.code.instructions.items(.data)[@intFromEnum(inst)].un_node;
     const src = block.nodeOffset(inst_data.src_node);
     const ptr = try sema.resolveInst(inst_data.operand);
+    const ptr_ty = sema.typeOf(ptr);
     try sema.checkNullableType(block, src, sema.typeOf(ptr).elemType2(zcu));
-    if ((try sema.resolveValue(ptr)) == null) {
-        return block.addUnOp(.is_non_null_ptr, ptr);
+    if (try sema.resolveValue(ptr)) |ptr_val| {
+        if (try sema.pointerDeref(block, src, ptr_val, ptr_ty)) |loaded_val| {
+            return sema.analyzeIsNull(block, Air.internedToRef(loaded_val.toIntern()), true);
+        }
     }
-    const loaded = try sema.analyzeLoad(block, src, ptr, src);
-    return sema.analyzeIsNull(block, src, loaded, true);
+    if (ptr_ty.childType(zcu).isNullFromType(zcu)) |is_null| {
+        return if (is_null) .bool_false else .bool_true;
+    }
+    return block.addUnOp(.is_non_null_ptr, ptr);
 }
 
 fn checkErrorType(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) !void {
@@ -32405,7 +32410,6 @@ fn analyzeSliceLen(
 fn analyzeIsNull(
     sema: *Sema,
     block: *Block,
-    src: LazySrcLoc,
     operand: Air.Inst.Ref,
     invert_logic: bool,
 ) CompileError!Air.Inst.Ref {
@@ -32421,15 +32425,10 @@ fn analyzeIsNull(
         return if (bool_value) .bool_true else .bool_false;
     }
 
-    const inverted_non_null_res: Air.Inst.Ref = if (invert_logic) .bool_true else .bool_false;
-    const operand_ty = sema.typeOf(operand);
-    if (operand_ty.zigTypeTag(zcu) == .optional and operand_ty.optionalChild(zcu).zigTypeTag(zcu) == .noreturn) {
-        return inverted_non_null_res;
+    if (sema.typeOf(operand).isNullFromType(zcu)) |is_null| {
+        const result = is_null != invert_logic;
+        return if (result) .bool_true else .bool_false;
     }
-    if (operand_ty.zigTypeTag(zcu) != .optional and !operand_ty.isPtrLikeOptional(zcu)) {
-        return inverted_non_null_res;
-    }
-    try sema.requireRuntimeBlock(block, src, null);
     const air_tag: Air.Inst.Tag = if (invert_logic) .is_non_null else .is_null;
     return block.addUnOp(air_tag, operand);
 }
@@ -33007,7 +33006,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_src, ptr, true);
+                    const is_non_null = try sema.analyzeIsNull(block, ptr, true);
                     try sema.addSafetyCheck(block, src, is_non_null, .unwrap_null);
                 }
 
@@ -33065,7 +33064,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_src, ptr, true);
+            const is_non_null = try sema.analyzeIsNull(block, ptr, true);
             try sema.addSafetyCheck(block, src, is_non_null, .unwrap_null);
         }
 
src/Type.zig
@@ -4114,6 +4114,16 @@ 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 neve `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 child = ty.optionalChild(zcu);
+    if (child.zigTypeTag(zcu) == .noreturn) return true; // `?noreturn` is always null
+    return null;
+}
+
 pub const @"u1": Type = .{ .ip_index = .u1_type };
 pub const @"u8": Type = .{ .ip_index = .u8_type };
 pub const @"u16": Type = .{ .ip_index = .u16_type };
test/behavior/optional.zig
@@ -498,6 +498,20 @@ test "optional of noreturn used with orelse" {
     try expect(val == 123);
 }
 
+test "mutable optional of noreturn" {
+    if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
+
+    var a: ?noreturn = null;
+    if (a) |*ptr| {
+        _ = ptr;
+        @compileError("bad");
+    } else {
+        // this is what we expect to hit
+        return;
+    }
+    @compileError("bad");
+}
+
 test "orelse on C pointer" {
 
     // TODO https://github.com/ziglang/zig/issues/6597
test/cases/compile_errors/branch_in_comptime_only_scope_uses_condbr_inline.zig
@@ -15,11 +15,9 @@ pub export fn entry2() void {
 }
 
 // error
-// backend=stage2
-// target=native
 //
 // :5:15: error: unable to evaluate comptime expression
 // :5:13: note: operation is runtime due to this operand
 // :4:72: note: '@shuffle' mask must be comptime-known
-// :13:11: error: unable to evaluate comptime expression
+// :13:11: error: unable to resolve comptime value
 // :12:72: note: '@shuffle' mask must be comptime-known