Commit 0b1dd845d9

Veikka Tuominen <git@vexu.eu>
2022-03-09 13:05:22
stage2: add error for non-void error union payload being ignored
See https://github.com/ziglang/zig/pull/6060#discussion_r471032912
1 parent b626977
src/AstGen.zig
@@ -2505,10 +2505,10 @@ fn addEnsureResult(gz: *GenZir, maybe_unused_result: Zir.Inst.Ref, statement: As
             .dbg_block_end,
             .ensure_result_used,
             .ensure_result_non_error,
+            .ensure_err_union_payload_void,
             .@"export",
             .export_value,
             .set_eval_branch_quota,
-            .ensure_err_payload_void,
             .atomic_store,
             .store,
             .store_node,
@@ -5492,6 +5492,7 @@ fn ifExpr(
                 try then_scope.addDbgVar(.dbg_var_val, ident_name, payload_inst);
                 break :s &payload_val_scope.base;
             } else {
+                _ = try then_scope.addUnNode(.ensure_err_union_payload_void, cond.inst, node);
                 break :s &then_scope.base;
             }
         } else if (if_full.payload_token) |payload_token| {
@@ -5829,6 +5830,7 @@ fn whileExpr(
                 dbg_var_inst = indexToRef(payload_inst);
                 break :s &payload_val_scope.base;
             } else {
+                _ = try then_scope.addUnNode(.ensure_err_union_payload_void, cond.inst, node);
                 break :s &then_scope.base;
             }
         } else if (while_full.payload_token) |payload_token| {
src/print_zir.zig
@@ -162,6 +162,7 @@ const Writer = struct {
             .load,
             .ensure_result_used,
             .ensure_result_non_error,
+            .ensure_err_union_payload_void,
             .ret_node,
             .ret_load,
             .resolve_inferred_alloc,
@@ -235,7 +236,6 @@ const Writer = struct {
 
             .ref,
             .ret_tok,
-            .ensure_err_payload_void,
             .closure_capture,
             .switch_capture_tag,
             => try self.writeUnTok(stream, inst),
src/Sema.zig
@@ -1034,8 +1034,8 @@ fn analyzeBodyInner(
                 i += 1;
                 continue;
             },
-            .ensure_err_payload_void => {
-                try sema.zirEnsureErrPayloadVoid(block, inst);
+            .ensure_err_union_payload_void => {
+                try sema.zirEnsureErrUnionPayloadVoid(block, inst);
                 i += 1;
                 continue;
             },
@@ -3100,6 +3100,33 @@ fn zirEnsureResultNonError(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com
     }
 }
 
+fn zirEnsureErrUnionPayloadVoid(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void {
+    const tracy = trace(@src());
+    defer tracy.end();
+
+    const inst_data = sema.code.instructions.items(.data)[inst].un_node;
+    const src = inst_data.src();
+    const operand = try sema.resolveInst(inst_data.operand);
+    const operand_ty = sema.typeOf(operand);
+    const err_union_ty = if (operand_ty.zigTypeTag() == .Pointer)
+        operand_ty.childType()
+    else
+        operand_ty;
+    // TODO this should be validated in a more generic instruction that is
+    // emitted for all ifs and whiles with an error union condition.
+    if (err_union_ty.zigTypeTag() != .ErrorUnion) return;
+    const payload_ty = err_union_ty.errorUnionPayload().zigTypeTag();
+    if (payload_ty != .Void and payload_ty != .NoReturn) {
+        const msg = msg: {
+            const msg = try sema.errMsg(block, src, "error union payload is ignored", .{});
+            errdefer msg.destroy(sema.gpa);
+            try sema.errNote(block, src, msg, "payload value can be explicitly ignored with '|_|'", .{});
+            break :msg msg;
+        };
+        return sema.failWithOwnedErrorMsg(msg);
+    }
+}
+
 fn zirIndexablePtrLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Inst.Ref {
     const tracy = trace(@src());
     defer tracy.end();
@@ -7681,24 +7708,6 @@ fn zirErrUnionCodePtr(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileE
     return block.addTyOp(.unwrap_errunion_err_ptr, result_ty, operand);
 }
 
-fn zirEnsureErrPayloadVoid(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void {
-    const tracy = trace(@src());
-    defer tracy.end();
-
-    const inst_data = sema.code.instructions.items(.data)[inst].un_tok;
-    const src = inst_data.src();
-    const operand = try sema.resolveInst(inst_data.operand);
-    const operand_ty = sema.typeOf(operand);
-    if (operand_ty.zigTypeTag() != .ErrorUnion) {
-        return sema.fail(block, src, "expected error union type, found '{}'", .{
-            operand_ty.fmt(sema.mod),
-        });
-    }
-    if (operand_ty.errorUnionPayload().zigTypeTag() != .Void) {
-        return sema.fail(block, src, "expression value is ignored", .{});
-    }
-}
-
 fn zirFunc(
     sema: *Sema,
     block: *Block,
src/Zir.zig
@@ -402,6 +402,8 @@ pub const Inst = struct {
         /// Emits a compile error if an error is ignored.
         /// Uses the `un_node` field.
         ensure_result_non_error,
+        /// Emits a compile error error union payload is not void.
+        ensure_err_union_payload_void,
         /// Create a `E!T` type.
         /// Uses the `pl_node` field with `Bin` payload.
         error_union_type,
@@ -646,9 +648,6 @@ pub const Inst = struct {
         /// Given a pointer to an error union value, returns the error code. No safety checks.
         /// Uses the `un_node` field.
         err_union_code_ptr,
-        /// Takes a *E!T and raises a compiler error if T != void
-        /// Uses the `un_tok` field.
-        ensure_err_payload_void,
         /// An enum literal. Uses the `str_tok` union field.
         enum_literal,
         /// A switch expression. Uses the `pl_node` union field.
@@ -1060,6 +1059,7 @@ pub const Inst = struct {
                 .elem_val_node,
                 .ensure_result_used,
                 .ensure_result_non_error,
+                .ensure_err_union_payload_void,
                 .@"export",
                 .export_value,
                 .field_ptr,
@@ -1113,7 +1113,6 @@ pub const Inst = struct {
                 .err_union_code_ptr,
                 .ptr_type,
                 .overflow_arithmetic_ptr,
-                .ensure_err_payload_void,
                 .enum_literal,
                 .merge_error_sets,
                 .error_union_type,
@@ -1282,7 +1281,7 @@ pub const Inst = struct {
                 .dbg_block_end,
                 .ensure_result_used,
                 .ensure_result_non_error,
-                .ensure_err_payload_void,
+                .ensure_err_union_payload_void,
                 .set_eval_branch_quota,
                 .atomic_store,
                 .store,
@@ -1615,6 +1614,7 @@ pub const Inst = struct {
                 .elem_val_node = .pl_node,
                 .ensure_result_used = .un_node,
                 .ensure_result_non_error = .un_node,
+                .ensure_err_union_payload_void = .un_node,
                 .error_union_type = .pl_node,
                 .error_value = .str_tok,
                 .@"export" = .pl_node,
@@ -1677,7 +1677,6 @@ pub const Inst = struct {
                 .err_union_payload_unsafe_ptr = .un_node,
                 .err_union_code = .un_node,
                 .err_union_code_ptr = .un_node,
-                .ensure_err_payload_void = .un_tok,
                 .enum_literal = .str_tok,
                 .switch_block = .pl_node,
                 .switch_cond = .un_node,
test/behavior/error.zig
@@ -7,7 +7,7 @@ const mem = std.mem;
 /// A more basic implementation of std.testing.expectError which
 /// does not require formatter/printing support
 fn expectError(expected_err: anyerror, observed_err_union: anytype) !void {
-    if (observed_err_union) {
+    if (observed_err_union) |_| {
         return error.TestExpectedError;
     } else |err| if (err == expected_err) {
         return; // Success
test/cases/compile_errors/non_void_error_union_payload_ignored.zig
@@ -0,0 +1,25 @@
+pub export fn entry1() void {
+    var x: anyerror!usize = 5;
+    if (x) {
+        // foo
+    } else |_| {
+        // bar
+    }
+}
+pub export fn entry2() void {
+    var x: anyerror!usize = 5;
+    while (x) {
+        // foo
+    } else |_| {
+        // bar
+    }
+}
+
+// error
+// backend=stage2
+// target=native
+//
+// :3:5: error: error union payload is ignored
+// :3:5: note: payload value can be explicitly ignored with '|_|'
+// :11:5: error: error union payload is ignored
+// :11:5: note: payload value can be explicitly ignored with '|_|'