zig icon indicating copy to clipboard operation
zig copied to clipboard

Unclear error message when passing wrong type to allocator.free

Open Jarred-Sumner opened this issue 2 years ago • 4 comments

Zig Version

0.11.0-dev.1393+38eebf3c4

Steps to Reproduce and Observed Output

Pass a struct to std.mem.Allocator.free(allocator, my_struct);

You get:

  • an error message that doesn't clearly say what went wrong
  • no stack trace with a line number saying where it happened
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/mem/Allocator.zig:296:45: error: access of union field 'Pointer' while field 'Struct' is active
    const Slice = @typeInfo(@TypeOf(memory)).Pointer;
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/builtin.zig:201:18: note: union declared here
pub const Type = union(enum) {
                 ^~~~~
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/mem/Allocator.zig:296:45: error: access of union field 'Pointer' while field 'Struct' is active
    const Slice = @typeInfo(@TypeOf(memory)).Pointer;
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/builtin.zig:201:18: note: union declared here
pub const Type = union(enum) {
                 ^~~~~
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/mem/Allocator.zig:296:45: error: access of union field 'Pointer' while field 'Struct' is active
    const Slice = @typeInfo(@TypeOf(memory)).Pointer;
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/builtin.zig:201:18: note: union declared here
pub const Type = union(enum) {
                 ^~~~~

Expected Output

Something like this would be better:

/Users/jarred/zig/0.11.0-dev.1393+38eebf3c4/files/lib/std/mem/Allocator.zig:296:45: error: free() expects a slice but received struct `my.type.name`
@compilerError("error: free() expects a slice but received struct `" ++ @typeName(@TypeOf(value)) ++ "`");
^ 

allocator.free(my_struct);
^
./my-app.zig:201:18: note: passed here

Jarred-Sumner avatar Jan 30 '23 10:01 Jarred-Sumner

@Vexu Does this need moving to the 0.11.0 milestone?

leoconst avatar Feb 12 '23 12:02 leoconst

Fix reverted in 3c2a43fdcc2d9aeafafe7ef37c7b805e18fac351.

andrewrk avatar Feb 12 '23 13:02 andrewrk

Fix reverted in 3c2a43f.

Makes sense, it did seem like a lot of code.

As this ended up as a patch with two things - "better error messages" and "pointer size restrictions" - is it worth raising a smaller PR (separate from this issue) for the latter, with the .One and .Slice restrictions in place (I guess with just an assert(Slice.size == ...), or are those also unwanted?

leoconst avatar Feb 12 '23 13:02 leoconst

Similar issue I had where the compiler gives no indication of where the error occurred.

The error:

user@ed5f6dedeaa1:/zig/zigbug2# zig build
/opt/zig/zig-linux-aarch64-0.11.0-dev.1912+12b74b2c0/lib/std/mem.zig:3622:9: error: expected []T or *[_]T, passed *main.MyStruct
        @compileError("expected []T or *[_]T, passed " ++ @typeName(sliceType));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/zig/zig-linux-aarch64-0.11.0-dev.1912+12b74b2c0/lib/std/mem.zig:3629:59: note: called from here
pub fn sliceAsBytes(slice: anytype) SliceAsBytesReturnType(@TypeOf(slice)) {
                                    ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
error: zigbug2...
error: The following command exited with error code 1:

The code:

const std = @import("std");

const MyStruct = struct {
    name: []u8,
};

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    const name = "Test";
    var s = allocator.create(MyStruct) catch |err| {
        return err;
    };
    s.name = allocator.alloc(u8, name.len) catch |err| {
        allocator.free(s);
        return err;
    };
    std.mem.copy(u8, s.name, name);
}

ringtailsoftware avatar Mar 12 '23 17:03 ringtailsoftware

It looks like the traces are short when the function signature itself causes a compile error. Each of these examples cause a compile error whose error trace doesn't lead all the way back to the callsite in main().

pub fn main() void {
    _ = badReturn(u32, 100); // line not included in error trace
    badParam(u32, 9876); // line not included in error trace
    badParamSwitch(100, false); // line not included in error trace
}

// When there is a compile error determining the return type
fn badReturn(comptime T: type, t: T) BadReturnInner(T) {
    return t + 1;
}

fn BadReturnInner(comptime T: type) type {
    if (T == u32) @compileError("bad return type");
    return T;
}

// When there is a compile error determining a parameter type
fn badParam(comptime T: type, comptime t: BadParamInner(T)) void {
    _ = t;
}

fn BadParamInner(comptime T: type) type {
    if (T == u32) @compileError("bad parameter type");
    return T;
}

// It can also happen with inline expressions, not just comptime function calls
fn badParamSwitch(comptime v: anytype, comptime x: switch (v) {
    100 => @compileError("bad parameter: 100 is not allowed"),
    else => bool,
}) void {
    _ = x;
}

hryx avatar Apr 18 '23 00:04 hryx

Oh. Aside from the "unclear error message" aspect, the stack trace issue is the same as #12969. Also possibly related to #1295.

hryx avatar Apr 18 '23 03:04 hryx

I seem to be running into the same issue on 0.10.1, but I'm a very fresh user of Zig and can't figure out what went wrong

Zireael07 avatar May 02 '23 12:05 Zireael07

I just had this occur in that I mistakenly called defer self.allocator.free(id) when I should've called defer id.deinit(). I agree that the error message should at least point to where the error occurred in my code with the stack.

edyu avatar Sep 30 '23 05:09 edyu

I just had this occur in that I mistakenly called defer self.allocator.free(id) when I should've called defer id.deinit(). I agree that the error message should at least point to where the error occurred in my code with the stack.

Thanks @edyu! I just hit the same issue and your comment probably saved me hours of painful debugging 🙇

alloveras avatar Dec 11 '23 12:12 alloveras