Parsing ZON with optional fields

Help
1

Hello, i would like to write zon in such a way that, i could omit struct fields and their value would default to whatever code i write for them

example

// when i parse this, i check if the optional bool is present in the zon.
// if not, default it to "false" 
const Foo = struct {
  required_int: u32 = 0,
  optional_bool: bool = false,
};

is it possible to do this with ZON? i have tried it briefly with std.zon.parse.fromSlice() but it fails with error.WrongType error, and the options parameter does not specify the ability to notify the parser that missing fields is OK and it should just report them instead of erroring out.

i have a ton of ZON to write where many fields are unnecessary to write and optionality would perfect for that.

1 Like
2

Hmmm. I think at present, the best way forward is unfortunately a bit more complex: you need something like

// has all required fields and no optional ones
const MinFoo = struct {
    required_int: u32,
};

// one struct with matching name per field,
// although surely if you're willing to go lower level with the parser
// this is overkill
const FooOptionals: []const type = &.{ struct { optional_bool: bool = false } };

// to be honest, the above could be comptime-generated from Foo
// based strictly on whether there is or is not a default value provided
pub const Foo = struct {
    required_int: u32,
    optional_bool: bool = false,

    pub fn fromZonSlice(
        allocator: std.mem.Allocator, 
        source: [:0]const u8, 
        diag: ?*Diagnostics, 
        options: Options,
    ) error{ OutOfMemory, ParseZon }!Foo {
        const min = try std.zon.parse.fromSlice(MinFoo, allocator, source, diag, options);
        var ret: Foo = undefined;
        inline for (fields) |name| { // pseudocode, forgive me, easy to fill in tho
            @field(ret, name) = @field(min, name);
        }
        inline for (FooOptionals) |T| {
            const optional: T = std.zon.parse.fromSlice(T, allocator, source, diag, .{
                .ignore_unknown_fields = true,
            }) catch .{};
            inline for (fields) |name| { // pseudocode again
                @field(ret, name) = @field(optional, name);
            }
        }
        return ret;
    }
};
3

Ah, so there is no builtin solution in the std for my specific use case?
That’s fine i suppose, i will write my own ZON field parser. thank you.

4

You’re correct that there’s not currently a feature for this, but IMO there should be. I didn’t consider this use case when I wrote std.zon.parse but since then I’ve also wanted it a few times.

I have a fork of std.zon.parse that adds exactly the functionality you’re asking for that I intend to upstream into std. I just wanted to live with it a little while to make sure I like the API before making the PR.

It’s been more than long enough that I should go ahead and make the PR. I’ll try to get it in for 0.17, but if you don’t want to wait feel free to copy this file into your project for the time being. The function you want is fromSliceDefaults, it’s of course possible this will change once I PR it:

If you have any trouble getting it working let me know. That’ll be useful for me so that I can resolve any issues when PRing it, but it should be stable I use it daily in mr_texture.

In case anyone else reading this is having trouble following–std.zon.parse does allow you to leave off fields if they have a default value, but that’s not what @devengel is asking about.

The goal here is to start with a default value–possibly runtime known or loaded from another config file or such–and then override any parts of it that are specified by a given piece of ZON.

If this is hard to follow, imagine you were designing a config file for a text editor.

Your editor might first load its options from ~/.config/mycooleditor.zon, and then override those with anything specified in $CWD/mycooleditor.zon. For this to work, when $CWD/mycooleditor.zon is loaded, it needs to leave any fields not listed in that file alone so they retain the user’s global settings.

I’ll come up with better language for this pattern when I make the PR.

18 Likes
5

I know this is solved, but in case you wanna experiment more (and maybe more complex yet) alternatives, this is how manifest file (build.zig.zon) is parsed using std.zig.Ast.parse: https://codeberg.org/ziglang/zig/src/commit/5d08e47160ade85f0b47f925f9aa32b66827e82e/lib/compiler/Maker/Package/Manifest.zig#L582

1 Like
6

Thank you, your provided code works great.
i did however had to create “zon” structs for the code to work, as the code doesn’t seem to be able to handle packed structs very well.

that is however a very minor issue and i don’t mind doing it.
i hope this comment helps with your PR.

1 Like
7

No problem! I’ll look into the packed struct issue tomorrow, feel free to share a repro if you have one though I have a hunch about what the problem is.

8

Sure! here is the problematic struct.

pub const Foo = packed struct(u32) {
    a: u5,
    b: u5,
    c: u5,
    d: u5,
    e: u5,
    f: u5,

    reserved: u2 = 0,

// here is the error, incase its helpful.
lib/std/zon/parse.zig:823:56: 
error: expected type '?*const u5', found '*align(4:0:4) const u5'
};
9

Thanks for the repro, will fix!

1 Like
10

Sorry for reviving the thread again, but i found another bug.

the code will panic on out of bounds, if there is a missing struct field from the zone.
thread 34719 panic: index out of bounds: index 596, len 595

something like

const Foo = struct {
    string1: []const u8,
    string2: []const u8,
    float: f32,

    int1: u16,
    int2: u16,
    int3: u16,

with the zon looking like

    .foo = .{
        .{
            .string1 = "a",
            .string2 = "b",
            // notice the missing float.
            .int1 = 3,
            .int2 = 12,
            .int3 = 1,
        },
    },
11

No problem at all, thanks for the report!

I ended up rewriting the whole thing today using a different approach. You can see the PR here, and give it a try if you’re targeting Zig master:

With this approach, you no longer pass in defaults. Instead you pass in a pointer to a value you want to update. For example, here’s mr_texture updated to use the new API, the important function is std.zon.parse.updateFromSlice:

// Read the config file(s)
var config: Texture.Options = .{};
for (args.named.config.items) |path| {
    // Get the config source file
    const src = cwd.readFileAllocOptions(
        io,
        path,
        gpa,
        .unlimited,
        .@"1",
        0,
    ) catch |err| {
        std.process.fatal("{s}: {s}", .{ path, @errorName(err) });
    };
    defer gpa.free(src);

    // Parse the ZON and update the config
    var diag: zon.Diagnostics = .{};
    defer diag.deinit(gpa);
    zon.updateFromSlice(
        Texture.Options,
        gpa,
        &config,
        src,
        &diag,
        .{},
    ) catch |err| switch (err) {
        error.OutOfMemory => return error.OutOfMemory,
        error.ParseZon => std.process.fatal("{s}: {f}", .{ path, diag }),
    };
}

I suspect this rewrite won’t have the same bug you just hit, let me know if you’re able to test it out.

If you’re not able to try it out (e.g. maybe you’re targeting Zig 0.16 or are too busy) no worries, let me know and I’ll look into it myself. In that case a full code snippet reproducing the problem would be useful though, because I’m not sure that I 100% understand the setup that lead to the panic.

2 Likes
12

No directly related to the topic but I wonder is there a way to concat slices when using updateFrom* functions? I’ve tested the PR version and it replaces .slice with new value. This is fine if global configuration is a set of defaults and each field must be overwritten.

13

Thanks for trying it out! You can replace individual fields e.g. on a struct, but as you noticed pointers are all or nothing.

There’s no current support in my branch for concatenating slices on update. The actual concatenation would be no problem, but the main challenge here is providing a way for a user to opt into this behavior. It shouldn’t really be a global option, since this is likely not how you want your string fields to work, and I don’t want to complicate the interface too much by adding a comptime list of fields to it or something.

That being said, while it’s not likely to make it into this PR, I do have an idea for a separate ZON feature that, incidentally, would allow for a caller to get this behavior.

A number of people have asked for the ability to provide a custom parseZon function on their types to override how ZON is parsed. While this functionality is clearly desirable, we’re generally moving away from “magic decls” that change the behavior of types when present, so I’ve been a bit stuck on the right way to implement it.

I need some time to get the details right, but I ran into the same issue when setting up some UI reflection code and came up with a solution I’m pretty happy with there. The gist of it is you provide a callback to the parser that gives the user a chance to switch on the type and provide their own handling, or opt into the defaults. With this could override just a specific field or specific sub type but leave everything else with the default behavior.

If I can adapt this idea to ZON, then there’d be a location for users to implement hooks to get behavior like this that’s too advanced to be built into the standard parser, without having to completely replace it. This would also mitigate a lot of the “ZON versions” of structs designed to avoid writing a custom parser.

3 Likes
14

thank you for the PR, i managed to convert the code to 0.16.0 and its working great.
just one weird issue im having, allocator is complaining about a memory leak inside the parser itself.

i supply diagnostics, so i thought diagnostics.deinit was enough.
it’s not? so i used std.zon.parse.free(allocator, zon_value), however, doing that makes it complain that there’s a double free somewhere within the std.zon.parse.free().

for the memory leak.

error(DebugAllocator): memory address 0x74da074603c0 leaked: 
lib/std/zon/parse.zig:937:35: 0x1592d04 in parseString__anon_120048 (std.zig)
            return aw.toOwnedSlice();
                                  ^
ib/std/zon/parse.zig:899:55: 0x158bdb3 in parseSlicePointer__anon_120029 (std.zig)
            .string_literal => return self.parseString(T, node),
                                                      ^
lib/std/zon/parse.zig:735:63: 0x15bb24b in parseExprInnerInto__anon_121519 (std.zig)
                        const new = try self.parseSlicePointer(@TypeOf(out.*), node);
                                                              ^
lib/std/zon/parse.zig:685:39: 0x15badf4 in parseExprInto__anon_115685 (std.zig)
        return self.parseExprInnerInto(node, out) catch |err| switch (err) {
                                      ^
ib/std/zon/parse.zig:1078:43: 0x168d3d4 in parseStructInto__anon_136368 (std.zig)
                    try self.parseExprInto(
                                          ^
ib/std/zon/parse.zig:747:41: 0x169e1d3 in parseExprInnerInto__anon_136303 (std.zig)
                try self.parseStructInto(node, out),
                                        ^
(additional stack frames may have been skipped...)
15

i do not know if its because of the 0.17 - 0.16 code conversion.
eitherway, i’m fine with this memory leak as it happens to memory thats allocated process start, and freed at process exit.

so if its my fault, please ignore me.

16

That would be amazing! i already have several ZON version structs and the number is growing lol.
thank you for your work.

1 Like
17

i pulled Mason’s patch to take a look and this doesn’t sound like an error in std to me: if I’m reading it correctly, the stack trace is trying to tell you that the caller owns this parsed string.

18

ah, ignore me then, apologies.
ill stop talking in this thread since its solved now.

19

@devengel no worries! The feedback is useful. If you supply a full code example I can check whether there’s a bug here.

After sleeping on this, I’m having second thoughts about this design as well. I’ve elaborated on them in the PR thread, I’m interested to hear feedback from other people who would use this feature. Feel free to reply here or on Codeberg:

(CC @knightpp: the comment linked above describes a slightly more verbose approach, but it would allow you to get the concatenation behavior you asked about today with no changes to std.)

1 Like
20

FIY some of your links are without target(empty).

Hmmm, I’ve tried your alternative approach. This works on 0.16. Yeah, maybe you can get away without updateFrom* functions. Cases where you can only use updateFrom* don’t immediately come to my mind.

const std = @import("std");
const Io = std.Io;

const Config = struct {
    const default_steps = 150;

    steps: u32 = default_steps,
    accumulator: u64 = 0,
    plugins: []u8 = &.{},
    tri_state: ?bool = null,

    fn update(self: *Config, arena: std.mem.Allocator, other: Config) !void {
        if (other.steps != default_steps) {
            self.steps = other.steps;
        }
        if (other.tri_state) |b| self.tri_state = b;

        self.accumulator += other.accumulator;
        self.plugins = try arena.realloc(self.plugins, self.plugins.len + other.plugins.len);
        @memcpy(self.plugins[self.plugins.len - other.plugins.len ..], other.plugins);
    }
};

pub fn main(init: std.process.Init) !void {
    const arena: std.mem.Allocator = init.arena.allocator();
    const io = init.io;

    var stdout_buffer: [1024]u8 = undefined;
    var stdout_file_writer: Io.File.Writer = .init(.stdout(), io, &stdout_buffer);
    const stdout_writer = &stdout_file_writer.interface;

    var v = try std.zon.parse.fromSliceAlloc(
        Config,
        arena,
        ".{.accumulator=1}",
        null,
        .{},
    );
    try stdout_writer.print("{any}\n", .{v});

    var other = try std.zon.parse.fromSliceAlloc(
        Config,
        arena,
        ".{.steps=12,.plugins=.{1,2},.tri_state=true}",
        null,
        .{},
    );
    try v.update(arena, other);
    try stdout_writer.print("{any}\n", .{v});

    other = try std.zon.parse.fromSliceAlloc(
        Config,
        arena,
        ".{.accumulator=5, .plugins=.{3,4},.tri_state=false}",
        null,
        .{},
    );
    try v.update(arena, other);
    try stdout_writer.print("{any}\n", .{v});

    try stdout_writer.flush(); // Don't forget to flush!
}

// outputs
// .{ .steps = 150, .accumulator = 1, .plugins = {  }, .tri_state = null }
// .{ .steps = 12, .accumulator = 1, .plugins = { 1, 2 }, .tri_state = true }
// .{ .steps = 12, .accumulator = 6, .plugins = { 1, 2, 3, 4 }, .tri_state = false }