Add support for Decl Literal in .zon

[biom4st3r]

I'm working on an mmo ran into this issue where I'd like to use decl literals in my config files. I have this struct

pub const Skill = struct {
    pub const Forestry: Skill = .{ .name = "Forestry" };
    name: []const u8,
};

I'm specifically not using an enum, because the Client executable isn't supposed to know what skills exist until they are told about them. I reference the canonical skill(.Forestry) on the server and I'd like to also use it in my configs to I don't need to reach in and canonicalize it after parsing

.{
    .Tree = .{
        .name = "Tree",
        .desc = "It looks kind of sticky.",
        .results = .{
            .{
                .action = .Chop,
                .chance = 0.2,
                .result = .{
                    .item_name = "Log",
                    .amount = 1,
                },
                .exp = .{
                    .{
                        .skill = .Forestry,
                        //.{.name = "Forestry"},
                        .amount = 20,
                     },

[alexrp]

cc @MasonRemaley @mlugg

[MasonRemaley]

If we add support for this to the std zon parser, we should make sure it works when importing zon as well

[SeanTUT]

This should probably be more in depth. Decl literals can exist on structs unions, enums, and even pointers to any of those things (or opaques). Additionally, we can probably save on eval branch quota by checking for the decl with @hasDecl instead of a for loop. (forget that last part, I forgot this check was happening at runtime!)

Anyways, we should also be validating that the decl is of the correct type before returning it, or else we will just have a compile error when parsing any type that has decls which aren't valid decl literals.

[biom4st3r]

Oh yeah I can add this for those also. Also I do check the type on line 811

[biom4st3r]

Opaques are not supported in zon files and pointers should be handled by parseExprInner. However the pointer won't be a reference to the Declaration with my current implementation and propagating that information might be tricky.

[biom4st3r]

I'm not sure of the proper way to implement this for @import. I have this inside LowerZon.lowerStruct, but I'm not sure how to get the Navs in pub_decls analyzed or if this is even the right stage of compilation to get them analyzed

        .enum_literal => |name| {
            // Ensure the incoming name is interned
            const decl_name = try ip.getOrPutString(
                self.sema.gpa,
                self.sema.pt.tid,
                name.get(self.file.zoir.?),
                .no_embedded_nulls,
            );
            for (ip.namespacePtr(struct_info.namespace).pub_decls.keys()) |decl| {
                const nav = ip.getNav(decl);
                if (!nav.typeOf(ip) == res_ty) continue;
                
                if (nav.name == decl_name) {
                    return self.sema.pt.zcu.navValue(decl);
                }
            }
            return error.WrongType;

I've tried just about every pub fn .*Nav.* and not sure where to go from here

[biom4st3r]

Sinon pointed me in the right direction. Importing zon files with Declaration Literals works now

[SeanTUT]

On second glance, this might need some more in-depth work because of pointer decl literals.

Decl literals also work for single-item pointers to namespaced-types. For instance,

const MyType = struct {
  const value_decl: MyType = .{};
  const pointer_decl: *const MyType = &.value_decl;
};

comptime {
  // This works for pointers to a container
  const ptr: *const MyType = .pointer_decl;
  
  // It also works when pointers to a container have stricter qualifiers than the actual declaration, so coercion might occur
  const coerced_ptr: *const volatile MyType = .pointer_decl;
    
  @compileLog(ptr, coerced_ptr);
}

So for pointer types, we would have to to a more complicated check to first, see if it is a single-item pointer to a container type (including opaque types), then second, check for declarations which can coerce into said pointer type, which itself would require some reflection rather than a simple T == U check.

Then there's also the matter of packed pointers, which are currently impossible to differentiate via reflection, so they would unfortunately have to be a known issue with std.zon until language support is added (I'm holding out for #24061!)

[biom4st3r]

Wouldn't this also apply to optionals?

[SeanTUT]

IIRC, this does not. I'll get back to you in a minute after testing that, though

[SeanTUT]

I was dead wrong! The following does work:

const MyStruct = struct {
    const val_decl: MyStruct = .{};
    const ptr_decl: *const MyStruct = &.val_decl;

    // Optionals can coerce from decl literals of their child types
    const opt_val_decl: ?MyStruct = .val_decl;
    const opt_ptr_decl: ?*const MyStruct = .ptr_decl;

    comptime {
        // Additionally, optionals can be decl literals in and of themselves
        const x: ?MyStruct = .opt_val_decl;
        const y: ?*const MyStruct = .opt_ptr_decl;
        @compileLog(x, y);
    }

    // This even applies to nested optionals
    // Thankfully we don't have to worry about that for ZON though.
    const opt_opt_val_decl: ??MyStruct = .opt_val_decl;
    const opt_opt_ptr_decl: ?*const ?*const MyStruct = &.opt_ptr_decl;
    comptime {
        const x: ??MyStruct = .opt_opt_val_decl;
        const y: ?*const ?*const MyStruct = .opt_opt_ptr_decl;
        @compileLog(x, y);
    }

    const V = @Vector(3, ?*const MyStruct);

    // Error unions too!
    // Suffice to say, this is more in-depth than I thought.
    const err_opt_val_decl: anyerror!?MyStruct = error.Foo;
    comptime {
        const x: anyerror!?MyStruct = .err_opt_val_decl;
        @compileLog(x);
    }
};
comptime {
    _ = MyStruct;
}

There may be examples of even more stuff I'm not aware of, but I'm going to take a bit to read up on how decl literals are coerced and hopefully get back to you with a code example of how one might handle this.

[SeanTUT]

Pointer decl-literals would also have to be handled here of course

[jeffective]

Is this a language proposal?

[biom4st3r]

Is this a language proposal?

No I don't think this as a language proposal. I don't see why a zon could use anon struct initialization(.{}), but they intentionally can't use decl literals. They are identical ways to init a field

[mlugg]

This is a language proposal, and one I have mixed opinions on; I intentionally chose not to allow this so far. Afraid I'm too tired to elaborate right now, but if someone pings me tomorrow I can explain...

[biom4st3r]

Yeah I'd love to hear your thoughts on this!

[mlugg]

The concern I have is that this change means there are multiple ZON expressions which parse to the same thing. Previously, this was not generally the case. You could sort of do it by writing numeric literals in different ways, e.g. 123456 vs 123_456 vs 123456.0 etc, but there's an important difference there: those ZON expressions are known to be equivalent without knowing the schema. In other words, if you see ZON files containing .{ .a = 123456 } and .{ .a = 123_456 }, you know that they parse to the same structure, without needing to know the schema. On the other hand, if you see ZON files containing .{ .a = .{ .something = 123 } } and .{ .a = .my_decl_literal }, you have literally no way to know if those are considered equivalent without knowing the schema. This would be the case anywhere that a decl/enum literal appeared; it effectively becomes an opaque identifier for some externally-defined data. For an object format, whose singular goal is communicating data, it seems pretty unfortunate to need external information to have any understanding of that data.

That was a bit of a ramble, sorry, but I hope it made sense?

I also just don't really see particularly compelling use cases for decl literals in ZON. Regarding your use case:

I'd like to also use it in my configs to I don't need to reach in and canonicalize it after parsing

It sounds to me like you really just want a custom parsing function on your Skill type. That's actually probably much better, since I suspect what you really want is to accept only the Skill values that exist (i.e. you probably don't want to allow the user to write .{ .name = "some_skill_that_doesnt_exist" }!), so you really want full control over how the config is parsed. It looks like std.zon.parse doesn't currently support providing custom parsing functions for specific types; I think it probably should, like how std.json does. Then you'd just have something like this on Skill:

pub fn zonParse(p: *std.zon.Parser, node: std.zig.Zoir.Node.Index) !Skill {
    const skill_name = switch (node.get(p.zoir)) {
        .enum_literal => |s| s.get(p.zoir),
        else => return p.failNode(node, "expected enum literal"),
    };
    inline for (@typeInfo(Skill).@"struct".decls) |decl| {
        if (@TypeOf(@field(Skill, decl.name) != Skill) continue;
        if (std.mem.eql(u8, skill_name, decl.name)) return @field(Skill, decl.name);
    }
    return p.failNode(node, "unknown skill name");
}

I think that enhancement would be much better.

[MasonRemaley]

I share this concern, however, I also have a concern with custom parse functions implemented as described.

I believe that the JSON parser is the only remaining place in the standard library that has the property that the presence or absence of a method can silently change the behavior of the API. Formatting, for example, was recently changed to remove this property--you now have to specify "{f}" explicitly for the formatter to call a custom format function, and if you specify "{f}" but the format method is missing you get a compiler error.

My concern with this property is that it's harder to understand code that has it, and naming the method incorrectly leads to code that compiles but behaves differently.

I haven't yet been able to think of a neat way to offer custom parsers per type without violating this property which is why std.parse doesn't have this feature yet, but I'm open to ideas.

[EDIT] An additional issue is that this strategy only lets you change the behavior when parsing types you control, and only allows one version of the override per type. Note that the new format api doesn't have this issue anymore either.

[MasonRemaley]

It's also worth noting--you can avoid the separate canonicalization step described in the motivating use case for this PR today by dropping down from std.zon.parse to working with std.zig.Zoir directly. I think this can make a lot of sense when you have specific needs like hiding part of the schema.

Of course, this is the most practical when you either need many such changes or your changes are all at top level of your schema, and it's less practical if you're trying to change the parse for a single field nested deeply in something that otherwise behaves normally.

[InKryption]

I would suggest a design that allows you to specify the encode/decode behavior at the call-site, i.e. out-of-band codecs. There's a lot of ways to design that, some more complex than others, but a pretty simple way of going about it would be an API like:

const parse_ctx: struct {
    pub fn canParse(comptime T: type) bool {
        return switch (T) {
            Skill => true,
            else => false,
        };
    }
    pub fn parse(
        ctx: @This(),
        gpa: std.mem.Allocator,
        ast: Ast,
        zoir: Zoir,
        node: Zoir.Node.Index,
        comptime T: type
    ) T {
        comptime std.debug.assert(canParse(T));
        switch (T) {
            Skill => {
                // parse the skill
            },
            else => comptime unreachable,
        }
    }
} = .{};
const result = try std.zon.parse.fromZoirNodeCtx(T, ast, zoir, node, diag, options, parse_ctx);

The main friction here would be how to manage the memory allocation here. Could also require a free method on the parse context that knows how to free the value for the purposes of an errdefer, but that's a deeper discussion that would require slightly more concrete context to bikeshed.

[biom4st3r]

I understand the want to limit duplicate encodings, but we already have n+1 equivalent encodings per default value.
struct{.a: u64 = 75, .b: u64 = 65}: .{} == .{.a = 75} == .{.a=75,.b=65}.

From my perspective Zon is already an opaque structure. Just by viewing a Zon file I don't know what any of it means or it's purposes without also consulting the Zig code that its handled by. This could also just be my misunderstanding of the intended use case for zon files.

I do think supporting custom parsers would be good enough for my use case with the bonus of keeping the implementation simple.

[biom4st3r]

Adding a custom_parser option to the options struct seems like a good way to handle it if we expose the parse* functions to be used by the developer