Is this a known issue?

During advent of code I wrote this function:

fn groupPebbles(groups: *std.ArrayList(PebbleGroup)) !void {
    std.mem.sortUnstable(PebbleGroup, groups.items, {}, PebbleGroup.lessThan);

    const e = groups.items[0];
    try groups.append(e);

    ...
}

This worked fine at the time of writing it. About a week later, however, I updated Zig and it started segfaulting:

Segmentation fault at address 0x7f8124999000
/home/markus/.local/share/zigup/0.14.0-dev.2568+42dac40b3/files/lib/std/array_list.zig:263:13: 0x103d6e9 in append (11)
            new_item_ptr.* = item;
            ^
/home/markus/Development/aoc2024/src/11/main.zig:73:22: 0x103d3bf in groupPebbles (11)
    try groups.append(e);
                     ^

I am pretty sure that this is not supposed to happen, and the workaround I found was to simply create a new pebble group manually from the old one (.{ .a = a, .b = b, ... }-style).

I mean it’s fairly obvious what is happening here: The compiler is optimizing const e away, which already existed only to avoid this issue before the compiler update. This leads to the code only accessing e (aka items[0] when the array may have been resized, which leads to a segfault.

Is this known behaveior?

There are actually two optimizations here, one is the one you describe, and the second one is the parameter reference optimization when calling the append function.

The compiler team is aware that these cause problems, and they plan to remove parameter reference optimization and only apply it when the compiler can prove that there are no side effects.

Another workaround would be to use a var instead of a const, vars will always create a copy:

var e = groups.items[0];
_ = &e; // Hack to hide unused mutability warning
1 Like

Thank you for the info!

I actually find it very impressive that the compiler managed to find so many ways to optimize here, just a shame it wasnt the place to do so.

See ATTACK of the KILLER FEATURES for an in-depth explanation.

1 Like

It’s very easy to optimize when it doesn’t have to be correct.

2 Likes