Commit b05e8d46ec

LemonBoy <thatlemon@gmail.com>
2019-05-10 19:28:13
Change the enum value allocation strategy
1 parent 655794f
Changed files (3)
src
test
src/analyze.cpp
@@ -2003,6 +2003,11 @@ static Error resolve_enum_zero_bits(CodeGen *g, ZigType *enum_type) {
     enum_type->abi_size = tag_int_type->abi_size;
     enum_type->abi_align = tag_int_type->abi_align;
 
+    BigInt bi_one;
+    bigint_init_unsigned(&bi_one, 1);
+
+    TypeEnumField *last_enum_field = nullptr;
+
     for (uint32_t field_i = 0; field_i < field_count; field_i += 1) {
         AstNode *field_node = decl_node->data.container_decl.fields.at(field_i);
         TypeEnumField *type_enum_field = &enum_type->data.enumeration.fields[field_i];
@@ -2028,76 +2033,58 @@ static Error resolve_enum_zero_bits(CodeGen *g, ZigType *enum_type) {
 
         AstNode *tag_value = field_node->data.struct_field.value;
 
-        // In this first pass we resolve explicit tag values.
-        // In a second pass we will fill in the unspecified ones.
         if (tag_value != nullptr) {
+            // A user-specified value is available
             ConstExprValue *result = analyze_const_value(g, scope, tag_value, tag_int_type, nullptr);
             if (type_is_invalid(result->type)) {
                 enum_type->data.enumeration.is_invalid = true;
                 continue;
             }
+
             assert(result->special != ConstValSpecialRuntime);
-            assert(result->type->id == ZigTypeIdInt ||
-                   result->type->id == ZigTypeIdComptimeInt);
-            auto entry = occupied_tag_values.put_unique(result->data.x_bigint, tag_value);
-            if (entry == nullptr) {
-                bigint_init_bigint(&type_enum_field->value, &result->data.x_bigint);
+            assert(result->type->id == ZigTypeIdInt || result->type->id == ZigTypeIdComptimeInt);
+
+            bigint_init_bigint(&type_enum_field->value, &result->data.x_bigint);
+        } else {
+            // No value was explicitly specified: allocate the last value + 1
+            // or, if this is the first element, zero
+            if (last_enum_field != nullptr) {
+                bigint_add(&type_enum_field->value, &last_enum_field->value, &bi_one);
             } else {
-                Buf *val_buf = buf_alloc();
-                bigint_append_buf(val_buf, &result->data.x_bigint, 10);
+                bigint_init_unsigned(&type_enum_field->value, 0);
+            }
 
-                ErrorMsg *msg = add_node_error(g, tag_value,
-                        buf_sprintf("enum tag value %s already taken", buf_ptr(val_buf)));
-                add_error_note(g, msg, entry->value,
-                        buf_sprintf("other occurrence here"));
+            // Make sure we can represent this number with tag_int_type
+            if (!bigint_fits_in_bits(&type_enum_field->value,
+                                     tag_int_type->size_in_bits,
+                                     tag_int_type->data.integral.is_signed)) {
                 enum_type->data.enumeration.is_invalid = true;
-                continue;
-            }
-        }
-    }
 
-    // Now iterate again and populate the unspecified tag values
-    BigInt next_maybe_unoccupied_index;
-    bigint_init_unsigned(&next_maybe_unoccupied_index, 0);
+                Buf *val_buf = buf_alloc();
+                bigint_append_buf(val_buf, &type_enum_field->value, 10);
+                add_node_error(g, field_node,
+                    buf_sprintf("enumeration value %s too large for type '%s'",
+                        buf_ptr(val_buf), buf_ptr(&tag_int_type->name)));
 
-    // Since we're allocating positive values only we have one less bit
-    // available if the tag type is signed (eg. for a i8 we can only use (0,127))
-    unsigned tag_bit_width = tag_int_type->size_in_bits;
-    if (tag_int_type->data.integral.is_signed)
-        tag_bit_width--;
+                break;
+            }
+        }
 
-    for (uint32_t field_i = 0; field_i < field_count; field_i += 1) {
-        AstNode *field_node = decl_node->data.container_decl.fields.at(field_i);
-        TypeEnumField *type_enum_field = &enum_type->data.enumeration.fields[field_i];
-        AstNode *tag_value = field_node->data.struct_field.value;
+        // Make sure the value is unique
+        auto entry = occupied_tag_values.put_unique(type_enum_field->value, field_node);
+        if (entry != nullptr) {
+            enum_type->data.enumeration.is_invalid = true;
 
-        // Already handled in the loop above
-        if (tag_value != nullptr)
-            continue;
+            Buf *val_buf = buf_alloc();
+            bigint_append_buf(val_buf, &type_enum_field->value, 10);
 
-        // Make sure we can represent this number with tag_int_type
-        const unsigned repr_bits = bigint_bits_needed(&next_maybe_unoccupied_index);
-        if (repr_bits > tag_bit_width) {
-            enum_type->data.enumeration.is_invalid = true;
-            add_node_error(g, field_node,
-               buf_sprintf("enumeration value %" ZIG_PRI_u64 " too large for type '%s'",
-                    bigint_as_unsigned(&next_maybe_unoccupied_index),
-                    buf_ptr(&tag_int_type->name)));
-            break;
+            ErrorMsg *msg = add_node_error(g, field_node,
+                    buf_sprintf("enum tag value %s already taken", buf_ptr(val_buf)));
+            add_error_note(g, msg, entry->value,
+                    buf_sprintf("other occurrence here"));
         }
 
-        if (occupied_tag_values.size() == 0) {
-            type_enum_field->value = next_maybe_unoccupied_index;
-            bigint_incr(&next_maybe_unoccupied_index);
-        } else {
-            for (;;) {
-                auto entry = occupied_tag_values.put_unique(next_maybe_unoccupied_index, field_node);
-                if (entry == nullptr)
-                    break;
-                bigint_incr(&next_maybe_unoccupied_index);
-            }
-            type_enum_field->value = next_maybe_unoccupied_index;
-        }
+        last_enum_field = type_enum_field;
     }
 
     enum_type->data.enumeration.zero_bits_loop_flag = false;
test/stage1/behavior/enum.zig
@@ -940,13 +940,25 @@ test "enum literal in array literal" {
 }
 
 test "signed integer as enum tag" {
-    const SignedEnum = enum (i2) {
+    const SignedEnum = enum(i2) {
         A0 = -1,
-        A1 =  0,
-        A2 =  1,
+        A1 = 0,
+        A2 = 1,
     };
 
     expect(@enumToInt(SignedEnum.A0) == -1);
-    expect(@enumToInt(SignedEnum.A1) ==  0);
-    expect(@enumToInt(SignedEnum.A2) ==  1);
+    expect(@enumToInt(SignedEnum.A1) == 0);
+    expect(@enumToInt(SignedEnum.A2) == 1);
+}
+
+test "enum value allocation" {
+    const LargeEnum = enum(u32) {
+        A0 = 0x80000000,
+        A1,
+        A2,
+    };
+
+    expect(@enumToInt(LargeEnum.A0) == 0x80000000);
+    expect(@enumToInt(LargeEnum.A1) == 0x80000001);
+    expect(@enumToInt(LargeEnum.A2) == 0x80000002);
 }
test/compile_errors.zig
@@ -12,6 +12,19 @@ pub fn addCases(cases: *tests.CompileErrorContext) void {
         "tmp.zig:3:32: error: cast discards const qualifier",
     );
 
+    cases.add(
+        "overflow in enum value allocation",
+        \\const Moo = enum(u8) {
+        \\    Last = 255,
+        \\    Over,
+        \\};
+        \\pub fn main() void {
+        \\  var y = Moo.Last;
+        \\}
+    ,
+        "tmp.zig:3:5: error: enumeration value 256 too large for type 'u8'",
+    );
+
     cases.add(
         "attempt to cast enum literal to error",
         \\export fn entry() void {
@@ -5383,8 +5396,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void {
         "tmp.zig:12:20: note: referenced here",
     );
 
-    cases.add(
-        "specify enum tag type that is too small",
+    cases.add("specify enum tag type that is too small",
         \\const Small = enum (u2) {
         \\    One,
         \\    Two,
@@ -5396,9 +5408,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void {
         \\export fn entry() void {
         \\    var x = Small.One;
         \\}
-    ,
-        "tmp.zig:6:5: error: enumeration value 4 too large for type 'u2'"
-    );
+    , "tmp.zig:6:5: error: enumeration value 4 too large for type 'u2'");
 
     cases.add(
         "specify non-integer enum tag type",
@@ -5506,8 +5516,8 @@ pub fn addCases(cases: *tests.CompileErrorContext) void {
         \\    var x = MultipleChoice.C;
         \\}
     ,
-        "tmp.zig:6:9: error: enum tag value 60 already taken",
-        "tmp.zig:4:9: note: other occurrence here",
+        "tmp.zig:6:5: error: enum tag value 60 already taken",
+        "tmp.zig:4:5: note: other occurrence here",
     );
 
     cases.add(