Skip to content

Allow extern tagged unions #1072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Hejsil opened this issue Jun 7, 2018 · 9 comments
Open

Allow extern tagged unions #1072

Hejsil opened this issue Jun 7, 2018 · 9 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Hejsil
Copy link
Contributor

Hejsil commented Jun 7, 2018

Sometimes you need to provide a tagged union through your C api. Currently, the best way to do this is to declare an extern struct with a tag, and a union member:

pub const S = extern struct {
    tag: u8,
    data: extern union {
        A: u16,
        B: u8,
    },
};

export fn b(ptr: *const S) u8 {
    return ptr.data.B;
}

But is there really any reason that extern tagged unions shouldn't work? We could make this:

pub const U = extern union(enum) {
    A: u8,
    B: u16,
};

export fn a(ptr: *const U) u8 {
    return ptr.A;
}

Equivalent to this header:

enum UEnum {
    UA,
    UB,
};

struct U {
    enum UEnum tag;
    union {
        uint8_t A;
        uint16_t B;
    };
};

uint8_t a(struct U *ptr);

The only problem I see here is that the "tag" has to be given some name in the C header. If this is a problem, we could introduce a syntax like this:

pub const U = extern union(tag: enum) {
    A: u8,
    B: u16,
};

This would give the tag a name, and make it accessible from both Zig and C.

@BraedonWooding
Copy link
Contributor

What's a use case of having a tag name, you can access it in Zig through the struct regardless and there is a standard naming procedure for the produced header? Is there ever a need for just the 'tag' and not the whole union.

@Hejsil
Copy link
Contributor Author

Hejsil commented Jun 7, 2018

In the C headers generated, a tag name is required for the union to be useful in C code.

@BraedonWooding
Copy link
Contributor

Yes, but I meant couldn't we just have a standard name like structName_tag?

@Hejsil
Copy link
Contributor Author

Hejsil commented Jun 7, 2018

Sure, we can do that. It was just an idea to avoid field name collisions completely while allowing the tag name to be something not ugly. Keep in mind, this feature is for C using a Zig lib. The users of your lib would appreciate id, tag or kind over SomeLongStructName_tag.

@BraedonWooding
Copy link
Contributor

Fair enough, I can see that being much more pleasant 👍

@Hejsil
Copy link
Contributor Author

Hejsil commented Jun 7, 2018

Ideas for how to set the tag name for extern unions are welcome :) I remember @andrewrk mentioning that all parameters to keywords should be <param> instead of (param) to be consistent with async. With that in mind, name: tag is not really a parameter but a declaration, so maybe we want it to be extern union(enum, "tag") instead.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jun 7, 2018
@andrewrk andrewrk added this to the 0.4.0 milestone Jun 7, 2018
@andrewrk andrewrk added the accepted This proposal is planned. label Jun 7, 2018
@andrewrk
Copy link
Member

andrewrk commented Feb 7, 2019

I'd like to reconsider this issue in light of #1922

@andrewrk andrewrk removed the accepted This proposal is planned. label Feb 7, 2019
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 19, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Apr 30, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Dec 9, 2019
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
@expikr
Copy link
Contributor

expikr commented Mar 29, 2024

extern struct {
    tag: enum(u8){
        a,b,c,
    },
    data: extern union(.tag){
        a: A,
        b: B,
        c: C,
    },
}

@Pyrolistical
Copy link
Contributor

Pyrolistical commented Mar 29, 2024

Just to be clear about about the problem, here is a comparison the status quo vs what is being proposed:

status quo

pub const S = extern struct {
    tag: enum(i32) {
        A,
        B,
    },
    data: extern union {
        A: u16,
        B: u8,
    },
};

const s: S = ...
switch (s.tag) {
    .A => {
        // use s.data.A, which could be undefined behavior
    },
    .B => {
        // use s.data.B, which could be undefined behavior
    },
}

proposed

pub const S = extern struct {
    tag: enum(i32) {
        A,
        B,
    },
    data: extern union(.tag) {  // the union(.tag) part is subject to change
        A: u16,
        B: u8,
    },
};

const s: S = ...
switch (s.data) {
    .A => |a| {
        // a is u16
    },
    .B => |b| {
        // b is u8
    },
}

@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

5 participants