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

Enum tag sizes are too large #598

Closed
scurest opened this issue Nov 8, 2017 · 10 comments
Closed

Enum tag sizes are too large #598

scurest opened this issue Nov 8, 2017 · 10 comments
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@scurest
Copy link
Contributor

scurest commented Nov 8, 2017

https://github.com/zig-lang/zig/blob/53b18c8542d6d40c3aa41e9d97290a4ca46eaa15/src/analyze.cpp#L1392

currently picks how big the tag for an enum should be by using bits_needed_for_unsigned on the number of fields. bits_needed_for_unsigned(x) returns the smallest number of bits needed to hold the value x (unless x is 0, in which case it somewhat confusingly returns 0). That means if there are eg. two variants in an enum, enum { A, B }, the tag is a u2, the smallest int that can hold 2.

For an enum with 256 variants, this means it will take 2 bytes instead of 1.

Trying to change this got me errors involving @enumTagName when running the tests.

@scurest
Copy link
Contributor Author

scurest commented Nov 8, 2017

Also, fields with a type like enum {} probably shouldn't count towards the total. enum { A, B: enum {} } should be zero-sized, right?

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Nov 8, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Nov 8, 2017
@andrewrk
Copy link
Member

andrewrk commented Nov 8, 2017

Also, fields with a type like enum {} probably shouldn't count towards the total. enum { A, B: enum {} } should be zero-sized, right?

enum {} is the same as void, which is the same as the type of the A field in this example. I think the type of this example should be u1 - we still have to know whether it's A or B. If the u1 is 0 it's A; if the u1 is 1 it's B. Same as if the example were enum {A, B}.

@andrewrk
Copy link
Member

andrewrk commented Nov 8, 2017

I think this should be: get_smallest_unsigned_int_type(g, field_count - 1);. Let me write some more tests and get a fix going.

@scurest
Copy link
Contributor Author

scurest commented Nov 9, 2017

void is the type with one element isn't it? enum {} should have zero elements, so since enum { A, B: enum {} } can never be B, it only has one inhabitant and should be zero-sized.

@andrewrk
Copy link
Member

andrewrk commented Nov 9, 2017

There's no way to initialize an enum {} because the syntax is EnumType.Field { value } and there are no fields. But it works the same way as void, and you can initialize void like this: {}

const Foo = enum { A, B: void };
const Bar = enum { A, B};

Foo and Bar are exactly the same. Foo can be B like this: const b = Foo.B

@scurest
Copy link
Contributor Author

scurest commented Nov 9, 2017

Why is enum {} the same as void? Algebraically, I expect enum {} to be the sum type with no terms, ie. 0, while void is 1.

@andrewrk andrewrk reopened this Nov 9, 2017
@andrewrk
Copy link
Member

andrewrk commented Nov 9, 2017

Hmm, I see what you're saying. I'm not sure zig is prepared to handle or should handle the algebraic concept of enum {}. We might just disallow that construct.

@scurest
Copy link
Contributor Author

scurest commented Nov 9, 2017

Actually 0 does already exists as noreturn, I guess. I think disallowing it is better than making it void.

Anyway, since enum {} isn't an empty type after all and you fixed the tag size, I'll close this 👍

@scurest scurest closed this as completed Nov 9, 2017
@thejoshwolfe
Copy link
Contributor

A type with 0 values and a type with 1 value are kinda the same. They both contain 0 bits of information.

void is a 0-bit type. an array of length 0 is a 0-bit type. a pointer to a 0-bit type is also a 0-bit type, and an array of 0-bit elements is a 0-bit type. (here an array being like [n]T, not a slice.)

a struct with 0 members would be a 0-bit type, if it were allowed (not sure if it is or not). an enum with 0 or 1 member would be a 0-bit type. a u0 would be a 0-bit type (whose value is always 0).

Types are first class objects at comptime, so something like a pointer to void might happen as a result of passing void as a type parameter. For example, a set can be defined as a map from T to void.

However, there's currently no way to add or delete struct members or enum members with comptime code. So it makes more sense for both of those 0-member situations to be compile errors. I can't think of any reason to explicitly make a struct or enum that's completely useless.

@scurest
Copy link
Contributor Author

scurest commented Nov 10, 2017

Just looking at "number of bits of information", a singleton and an uninhabited type differ when they compose. struct { x: i32, y: Z } has 32 bits of information when Z has one value, and 0 bits of information when Z has zero values. enum { x: i32, y: Z } has slightly more than 32 bits of information when Z has one value, but exactly 32 when it has zero.

You can use them in generic code. eg. if consumers of a generic feature must provide a type for what kind of errors they produce, a consumer can provide an empty type to signal that it never produces errors.

Unless they really make the implementation difficult, I think they should be allowed. struct {} currently works correctly so I don't think it should be changed. It's the same as void which already exists anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

3 participants