Skip to content
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

translate-c: Remove usage of extern enum #9164

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

ehaas
Copy link
Contributor

@ehaas ehaas commented Jun 19, 2021

If the enum type cannot be translated for some reason, omit it. In that
case the previous behavior will occur - the enum constant's type will be
the enum's tag type.

Fixes #9153

@ehaas
Copy link
Contributor Author

ehaas commented Jun 19, 2021

This fixes the immediate issue in #9153 but does not completely remove the use of extern enum as discussed here #2115 (comment)

If desired, I could address that in this PR as well.

@Vexu
Copy link
Member

Vexu commented Jun 20, 2021

If desired, I could address that in this PR as well.

I think that would be preferable, no reason to fix something that is going to be removed.

@ehaas ehaas force-pushed the translate-c-enum-constants branch from 832a6ee to 279865f Compare June 21, 2021 22:35
@ehaas ehaas changed the title translate-c: Declare type of enum constants translate-c: Remove usage of extern enum Jun 21, 2021
Translate enum types as the underlying integer type. Translate enum constants
as top-level integer constants of the correct type (which does not necessarily
match the enum integer type).

If an enum constant's type cannot be translated for some reason, omit it.

See discussion ziglang#2115 (comment)

Fixes ziglang#9153
@ehaas ehaas force-pushed the translate-c-enum-constants branch from 279865f to 9ea73a2 Compare June 22, 2021 00:49
@ehaas
Copy link
Contributor Author

ehaas commented Jun 22, 2021

I believe this also fixes #5656

@Vexu Vexu linked an issue Jun 22, 2021 that may be closed by this pull request
@Vexu Vexu merged commit 0e7897a into ziglang:master Jun 23, 2021
@ehaas ehaas deleted the translate-c-enum-constants branch June 23, 2021 06:39
@kmeaw
Copy link

kmeaw commented Feb 15, 2022

Is it still possible to get former behavior? I am trying to convert integer values back to symbolic names.

const key = @tagName(@intToEnum(c.my_enum, 123));

@InKryption
Copy link
Contributor

@kmeaw C enums are translated as individual declarations now, so no, not really possible achieve the old behavior.

@kmeaw
Copy link

kmeaw commented Feb 15, 2022

It is possible to iterate over these declarations to build (either in run-time or in compile-time) a map of keys-to-values? In my case all of them have a unique common prefix (like E_ in E_FIRST = 121, E_SECOND = 122, E_THIRD = 123, ...).

@InKryption
Copy link
Contributor

@kmeaw Well, it's possible to do so, though not very practical, as you'd have to use std.meta.declarations(), which would most likely result in referencing declarations replaced by @compileErrors; that's not even to mention how slow that would make compilation as the file gets bigger.

One thing to consider is whether it might be beneficial to make a PR to make translate-c put enums into a namespace, though that could very likely result in some extra complexity to handle the case of when those enums are referenced by other translated code.

As it stands, you're probably better off just manually creating an enum based off of the declarations.

@ehaas
Copy link
Contributor Author

ehaas commented Feb 15, 2022

Is it still possible to get former behavior? I am trying to convert integer values back to symbolic names.

const key = @tagName(@intToEnum(c.my_enum, 123));

The reason translate-c can't do this in general is that C enumerator values are not required to be unique within an enum declaration, e.g. the following is valid C:

enum E {
    Foo = 1,
    Bar = 1,
};

There are other issues with trying to map C enums onto Zig enums, but I don't think they're relevant for your use case. I would also guess that 99% of real-world C code does use unique enumerator values, so perhaps there would be interest in extending translate-c to keep track of the values within an enum and create a non-exhaustive enum if they're unique.

@kmeaw
Copy link

kmeaw commented Feb 15, 2022

@InKryption, thank you for your suggestions. I had attempted to iterate over @typeInfo(c).Struct.decls but have hit some limits - comptime { for {} } crashes the compiler, inline for {} stops on unions I don't want to process. inline for { comptime { if {} }} fails because the declaration name is not a constant (but a loop variable), inline for { comptime { if {} }} exceeds 1000 backward branches. So I suppose the only option I have is taking a reversal of this commit and adding it to bring C enums back but in a separate struct namespace (enum?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

translate-c: enum constants can have incorrect type Enums aren't cast from ints in macros with translate-c
4 participants