Surprising behavior when consuming payload of tagged union passed to switch

jbe · June 27, 2025, 12:51pm

I came across this curious behavior:

const std = @import("std");

const Datum = struct {
    pub fn init() @This() {
        return @This(){};
    }
    pub fn deinit(self: *@This()) void {
        self.* = undefined;
    }
};

const Tag = enum { poor, rich };

const Choice = union(Tag) {
    poor: void,
    rich: Datum,
};

pub fn generate() Choice {
    return Choice { .rich = Datum.init() };
}

pub fn main() void {
    switch (generate()) {
        .rich => |*d| d.deinit(),
        .poor => {},
    }
}

Resulting in:

demo.zig:25:24: error: expected type '*demo.Datum', found '*const demo.Datum'
        .rich => |*d| d.deinit(),
                      ~^~~~~~~

(Zig version 0.15.0-dev.847+850655f06)

However, if I store the result of generate() in a temporary variable, then it works:

 pub fn main() void {
-    switch (generate()) {
+    var x = generate();
+    switch (x) {
         .rich => |*d| d.deinit(),
         .poor => {},
     }
 }

Is that intended? How to rewrite main without using a temporary variable in a reasonable way?

Justus2308 · June 27, 2025, 1:36pm

Yes. |*d| is a const pointer because values are const by default in Zig, so unless you explicitly declare the result of generate() as var (which is not possible if you directly evaluate it inside the switch condition) all references to it will also be const references.

You could have deinit() take a const pointer self: *const @This() or a value self: @This(). I don’t know if that’s possible with your real Datum type which I suspect looks different than the one in the example but if it is that would work. Note that the self.* = undefined is kind of useless here anyway (at least if you defer d.deinit()) because the generated datum goes out of scope as soon as the switch expression finishes.

I would just use a temporary variable, if you’re worried about polluting your scope you can always put a block around it.

rpkak · June 27, 2025, 1:40pm

A bit related:

github.com/ziglang/zig

Proposal: make by-ref capture semantics consistent

opened 09:53PM - 08 Apr 25 UTC

mlugg

breaking proposal accepted

## Background In Zig, several constructs can capture a value "by reference" usi…ng the syntax `|*x|`. The captures which can be by-ref are: * The capture of an iterable `for` operand (i.e. not a range) * The payload capture of a `if` on an optional or error union * The payload capture of a `while` on an optional or error union * The payload capture of a `switch` prong However, there is an inconsistency in how these by-ref captures work between `for` operands and other constructs. The inconsistency is made clear by these two examples: ```zig var arr: [5]u32 = undefined; for (arr) |*x| x.* = 0; ``` ```zig var opt: ?u32 = 123; if (opt) |*x| x.* = 456; ``` The second of these snippets compiles; the first does not. For the first snippet to compile, the `for` operand needs to be `&arr`. This difference occurs because in the latter case, Zig effectively inserts an `&` before the operand when the capture is "by-ref". `while` and `switch` captures also follow the latter behavior. The `for` behavior has some advantages. Mainly, it is more explicit; you can tell exactly what the code is doing. Implicit references can look a bit weird in a few cases. Consider: ```zig while (it.next()) |*x| { // ... } ``` What's that snippet doing? You'd be forgiven for thinking that `next` returns a `*?T` or similar, but actually, this inserts an implicit `&`, meaning `x` is a pointer to a *local temporary*. If `next` did return `*?T`, you'd currently have to write: ```zig while (it.next().*) |*x| { // ... } ``` Okay, so we're dereferencing the pointer... but we somehow get a pointer out which is derived from it? The logic here *does* make sense, but it's quite weird. The implicit-ref behavior also meshes particularly strangely with `switch`, because *any prong* having a by-ref capture changes how the operand is evaluated. This means there can be arbitrary distance between the operand and the code which decides how it is evaluated, which is pretty strange. ## Proposal Remove the implicit-ref behavior of `if`, `while`, and `switch` on their operands. These constructs will now accept pointers to their operand types, and pointers will be required for by-ref captures. Here's how this looks in practice: ```zig var opt: ?u32 = 123; if (&opt) |*x| x.* = 456; // this is also allowed, although redundant, when you capture by value if (&opt) |x| _ = x; // and likewise for error unions var eu: anyerror!u32 = 123; if (&eu) |*x| { x.* = 456; } else |_| {} if (&eu) |x| { _ = x; // by-val capture even though the operand is a pointer } else |_| {} // `while` works exactly like `if` // `switch` looks like this: const U = union(enum) { foo: u32, bar: u64, }; var u: U = .{ .x = 123 }; switch (&u) { .foo => |*x| x.* = 456, .bar => |y| _ = y, } // again, it's okay for the captures to be by-val switch (&u) { .foo => |x| _ = x, .bar => |y| _ = y, } ``` There's one problem here: `switch (&u)` already has a different meaning! Currently, `switch` on pointers is allowed, and switches on the *address*. **As such, this proposal also proposes removing `switch` on pointers as it works today.** If desired, the exact same behavior can be achieved by switching on `@intFromPtr` of a pointer. (I don't think `switch` on pointers is heavily relied upon in the wild; my source for this is that it's pretty much completely broken on the LLVM backend right now!)

swenninger · June 27, 2025, 1:41pm

Also I think the usage of @This() is a bit overkill here.

jbe · June 27, 2025, 3:20pm

I read, for exampe in this article, that it is a common idiom to invalidate the memory of a structure at the end of a deinit function by setting self.* = undefined. For that, I need self to be a pointer to a non-const struct. In my case it is something like this:

pub const ConnectFail = struct {
    code: DbConnError,
    pg_conn: ?*c.PGconn,
    pub fn message(self: ConnectFail) [*:0]const u8 {
        return c.PQerrorMessage(self.pg_conn orelse return "libpq could not allocate memory");
    }
    pub fn deinit(self: *ConnectFail) void {
        if (self.pg_conn) |conn| {
            c.PQfinish(conn);
        }
        self.* = undefined;   
    }
};

(Having experimentally modified my approach here to refrain from demanding a message handler but returning a “to-be-deinit(ed)” error struct on failure.)

I then have to write something like that when using this code:

pub fn main() !void {
    const params = mylib.ConnectParams{ .string = "dbname=jbe" };
    var attempt = mylib.ConnectAttempt.init(params);
    switch (attempt) {
        .ok => |*conn| {
            defer conn.deinit();
            // use conn here
        },
        .fail => |*fail| {
            defer fail.deinit();
            std.debug.print("{s}\n", .{fail.message()});
        },
    }
}

So what I attempted was basically to have a switch statement that differentiates between the two outcomes of the connection attempt (successful connect and non-succeessful connection with a to-be-free’d error status).

Justus2308 · June 27, 2025, 3:52pm

Yeah, it’s a common idiom for sure, and if you use a temporary variable that is still potentially accessable after its deinitialization I would definitely use it.

I would suggest something like this:

const ConnectAttempt = union {
    ok: Connection,
    fail: ConnectFail,

    pub fn init(params: ConnectParams) ConnectAttempt { ... }

    pub fn deinit(attempt: *ConnectAttempt) void {
        switch (attempt) {
            .ok => |*conn| => conn.deinit(),
            .fail => |*fail| => fail.deinit(),
        }
    }
};

pub fn main() !void {
    const params = mylib.ConnectParams{ .string = "dbname=jbe" };
    var attempt = mylib.ConnectAttempt.init(params);
    defer attempt.deinit();
    switch (attempt) {
        .ok => |*conn| {
            // use conn here
        },
        .fail => |*fail| {
            std.debug.print("{s}\n", .{fail.message()});
        },
    }
}

This way initialization and deinitialization happen in the same scope right next to each other which helps avoid bugs and confusion IMO especially if you plan to add a lot of code to the switch prongs or more cases in the future.

jbe · June 27, 2025, 5:25pm

My idea was that maybe the error has a different lifetime than the connection handle. But right now I’m just experimenting and this isn’t a real-world problem yet. Still trying to get accustomed to the language.

Maybe I’ll just return a database handle both on success and failure (like the underlaying libpq), which is similar to your last proposal where the same deinit is used for the success and failure case.