zig icon indicating copy to clipboard operation
zig copied to clipboard

zig crash compiling memset of zero-length initializer (`@memset(list, .{})`)

Open rootbeer opened this issue 2 years ago • 4 comments

Zig Version

0.12.0-dev.1836+dd189a354

Steps to Reproduce and Observed Behavior

// emptystruct.zig
const std = @import("std");

const StructInProgress = struct {
    // x: usize, // add this field to hide the crash
};

pub fn main() !void {
    const allocator = std.heap.page_allocator;

    const list = try allocator.alloc(StructInProgress, 3);
    @memset(list, .{
        // .x = 0,
    });

    allocator.free(list);
}
$ zig build-exe emptystruct.zig

Panics due to this assert tripping in src/value.zig:

    pub fn hasRepeatedByteRepr(val: Value, ty: Type, mod: *Module) !?u8 {
        const abi_size = std.math.cast(usize, ty.abiSize(mod)) orelse return null;
        assert(abi_size >= 1);

Changing the assert to a runtime check (if (abi_size == 0) return null;) allows the compile to succeed, but I'm not sure if its the right thing to do, or if there are more subtle implications.

Expected Behavior

I'm not sure if this code should compile (if an array of zero-byte objects is zero bytes, initializing it with a zero-byte initializer is okay?) or be flagged as an error. (I ran into this while developing new code, and I was going to get around to filling in this structure once the code around it ran...)

AFAICT, none of the existing open issues cover this, but it seems familiar.

The assert was introduced in #15481 (8 months ago).

rootbeer avatar Dec 21 '23 06:12 rootbeer

i can't reproduce this with a little older versions (1828, 1835)

alichraghi avatar Dec 22 '23 00:12 alichraghi

(to members) should the resolution of this be noop or a compile error?

nektro avatar Dec 22 '23 01:12 nektro

From what I can tell: list is of type []StructInProgress. Assigning list[0] = .{}; is valid since .{} is a valid struct initializer for StructInProgress. (If the type had fields with default values, such a coercion should correctly initialize them as well.)

The documentation of @memset(dest, elem) clearly states that dest [may] be a mutable slice, and that elem is coerced to the element type of dest (meaning the second argument's result location should be StructInProgress, allowing its initialization via initializer). It currently even expressly states that dest may have any element type (no clause about 0-sized types or anything). Thus I don't see any particular reason for why this should be disallowed by compile error.

rohlem avatar Dec 23 '23 10:12 rohlem

I don't see a good reasoning for this being a noop. IMO, @memset(dest, @as(std.meta.Elem(@TypeOf(dest)), v)) ought to be semantically equivalent to @memset(dest, v).

notcancername avatar Dec 23 '23 23:12 notcancername