Commit 818a0a2629

Andrew Kelley <superjoe30@gmail.com>
2017-05-07 18:07:35
switch expression - add compile errors
* for duplicate integer value * for missing integer values * for missing else prong see #43
1 parent 29beb60
src/analyze.cpp
@@ -3851,25 +3851,30 @@ int64_t min_signed_val(TypeTableEntry *type_entry) {
     }
 }
 
+void eval_min_max_value_int(CodeGen *g, TypeTableEntry *int_type, BigNum *bignum, bool is_max) {
+    assert(int_type->id == TypeTableEntryIdInt);
+    if (is_max) {
+        if (int_type->data.integral.is_signed) {
+            int64_t val = max_signed_val(int_type);
+            bignum_init_signed(bignum, val);
+        } else {
+            uint64_t val = max_unsigned_val(int_type);
+            bignum_init_unsigned(bignum, val);
+        }
+    } else {
+        if (int_type->data.integral.is_signed) {
+            int64_t val = min_signed_val(int_type);
+            bignum_init_signed(bignum, val);
+        } else {
+            bignum_init_unsigned(bignum, 0);
+        }
+    }
+}
+
 void eval_min_max_value(CodeGen *g, TypeTableEntry *type_entry, ConstExprValue *const_val, bool is_max) {
     if (type_entry->id == TypeTableEntryIdInt) {
         const_val->special = ConstValSpecialStatic;
-        if (is_max) {
-            if (type_entry->data.integral.is_signed) {
-                int64_t val = max_signed_val(type_entry);
-                bignum_init_signed(&const_val->data.x_bignum, val);
-            } else {
-                uint64_t val = max_unsigned_val(type_entry);
-                bignum_init_unsigned(&const_val->data.x_bignum, val);
-            }
-        } else {
-            if (type_entry->data.integral.is_signed) {
-                int64_t val = min_signed_val(type_entry);
-                bignum_init_signed(&const_val->data.x_bignum, val);
-            } else {
-                bignum_init_unsigned(&const_val->data.x_bignum, 0);
-            }
-        }
+        eval_min_max_value_int(g, type_entry, &const_val->data.x_bignum, is_max);
     } else if (type_entry->id == TypeTableEntryIdFloat) {
         zig_panic("TODO analyze_min_max_value float");
     } else if (type_entry->id == TypeTableEntryIdBool) {
src/analyze.hpp
@@ -84,6 +84,7 @@ void complete_enum(CodeGen *g, TypeTableEntry *enum_type);
 bool ir_get_var_is_comptime(VariableTableEntry *var);
 bool const_values_equal(ConstExprValue *a, ConstExprValue *b);
 void eval_min_max_value(CodeGen *g, TypeTableEntry *type_entry, ConstExprValue *const_val, bool is_max);
+void eval_min_max_value_int(CodeGen *g, TypeTableEntry *int_type, BigNum *bignum, bool is_max);
 int64_t min_signed_val(TypeTableEntry *type_entry);
 uint64_t max_unsigned_val(TypeTableEntry *type_entry);
 
src/ir.cpp
@@ -12,6 +12,7 @@
 #include "ir_print.hpp"
 #include "os.hpp"
 #include "parseh.hpp"
+#include "range_set.hpp"
 
 struct IrExecContext {
     ConstExprValue *mem_slot_list;
@@ -12981,7 +12982,7 @@ static TypeTableEntry *ir_analyze_instruction_check_switch_prongs(IrAnalyze *ira
 
     if (switch_type->id == TypeTableEntryIdEnumTag) {
         TypeTableEntry *enum_type = switch_type->data.enum_tag.enum_type;
-        size_t *field_use_counts = allocate<size_t>(enum_type->data.enumeration.src_field_count);
+        AstNode **field_prev_uses = allocate<AstNode *>(enum_type->data.enumeration.src_field_count);
         for (size_t range_i = 0; range_i < instruction->range_count; range_i += 1) {
             IrInstructionCheckSwitchProngsRange *range = &instruction->ranges[range_i];
 
@@ -13011,24 +13012,65 @@ static TypeTableEntry *ir_analyze_instruction_check_switch_prongs(IrAnalyze *ira
             }
 
             for (size_t field_index = start_index; field_index <= end_index; field_index += 1) {
-                field_use_counts[field_index] += 1;
-                if (field_use_counts[field_index] > 1) {
+                AstNode *prev_node = field_prev_uses[field_index];
+                if (prev_node != nullptr) {
                     TypeEnumField *type_enum_field = &enum_type->data.enumeration.fields[field_index];
-                    ir_add_error(ira, start_value,
+                    ErrorMsg *msg = ir_add_error(ira, start_value,
                         buf_sprintf("duplicate switch value: '%s.%s'", buf_ptr(&enum_type->name),
                             buf_ptr(type_enum_field->name)));
+                    add_error_note(ira->codegen, msg, prev_node, buf_sprintf("other value is here"));
                 }
+                field_prev_uses[field_index] = start_value->source_node;
             }
         }
         for (uint32_t i = 0; i < enum_type->data.enumeration.src_field_count; i += 1) {
-            if (field_use_counts[i] == 0) {
+            if (field_prev_uses[i] == nullptr) {
                 ir_add_error(ira, &instruction->base,
                     buf_sprintf("enumeration value '%s.%s' not handled in switch", buf_ptr(&enum_type->name),
                         buf_ptr(enum_type->data.enumeration.fields[i].name)));
             }
         }
+    } else if (switch_type->id == TypeTableEntryIdInt) {
+        RangeSet rs = {0};
+        for (size_t range_i = 0; range_i < instruction->range_count; range_i += 1) {
+            IrInstructionCheckSwitchProngsRange *range = &instruction->ranges[range_i];
+
+            IrInstruction *start_value = range->start->other;
+            if (type_is_invalid(start_value->value.type))
+                return ira->codegen->builtin_types.entry_invalid;
+
+            IrInstruction *end_value = range->end->other;
+            if (type_is_invalid(end_value->value.type))
+                return ira->codegen->builtin_types.entry_invalid;
+
+            ConstExprValue *start_val = ir_resolve_const(ira, start_value, UndefBad);
+            if (!start_val)
+                return ira->codegen->builtin_types.entry_invalid;
+
+            ConstExprValue *end_val = ir_resolve_const(ira, end_value, UndefBad);
+            if (!end_val)
+                return ira->codegen->builtin_types.entry_invalid;
+
+            AstNode *prev_node = rangeset_add_range(&rs, &start_val->data.x_bignum, &end_val->data.x_bignum,
+                    start_value->source_node);
+            if (prev_node != nullptr) {
+                ErrorMsg *msg = ir_add_error(ira, start_value, buf_sprintf("duplicate switch value"));
+                add_error_note(ira->codegen, msg, prev_node, buf_sprintf("previous value is here"));
+                return ira->codegen->builtin_types.entry_invalid;
+            }
+        }
+        BigNum min_val;
+        eval_min_max_value_int(ira->codegen, switch_type, &min_val, false);
+        BigNum max_val;
+        eval_min_max_value_int(ira->codegen, switch_type, &max_val, true);
+        if (!rangeset_spans(&rs, &min_val, &max_val)) {
+            ir_add_error(ira, &instruction->base, buf_sprintf("switch must handle all possibilities"));
+            return ira->codegen->builtin_types.entry_invalid;
+        }
     } else {
-        // TODO check prongs of types other than enumtag
+        ir_add_error(ira, &instruction->base,
+            buf_sprintf("else prong required when switching on type '%s'", buf_ptr(&switch_type->name)));
+        return ira->codegen->builtin_types.entry_invalid;
     }
     ir_build_const_from(ira, &instruction->base);
     return ira->codegen->builtin_types.entry_void;
src/range_set.cpp
@@ -0,0 +1,81 @@
+#include "range_set.hpp"
+
+AstNode *rangeset_add_range(RangeSet *rs, BigNum *first, BigNum *last, AstNode *source_node) {
+    for (size_t i = 0; i < rs->src_range_list.length; i += 1) {
+        RangeWithSrc *range_with_src = &rs->src_range_list.at(i);
+        Range *range = &range_with_src->range;
+        if ((bignum_cmp_gte(first, &range->first) && bignum_cmp_lte(first, &range->last)) ||
+            (bignum_cmp_gte(last, &range->first) && bignum_cmp_lte(last, &range->last)))
+        {
+            return range_with_src->source_node;
+        }
+    }
+    rs->src_range_list.append({{*first, *last}, source_node});
+
+    return nullptr;
+
+}
+
+static bool add_range(ZigList<Range> *list, Range *new_range, BigNum *one) {
+    for (size_t i = 0; i < list->length; i += 1) {
+        Range *range = &list->at(i);
+
+        BigNum first_minus_one;
+        if (bignum_sub(&first_minus_one, &range->first, one))
+            zig_unreachable();
+
+        if (bignum_cmp_eq(&new_range->last, &first_minus_one)) {
+            range->first = new_range->first;
+            return true;
+        }
+
+        BigNum last_plus_one;
+        if (bignum_add(&last_plus_one, &range->last, one))
+            zig_unreachable();
+
+        if (bignum_cmp_eq(&new_range->first, &last_plus_one)) {
+            range->last = new_range->last;
+            return true;
+        }
+    }
+    list->append({new_range->first, new_range->last});
+    return false;
+}
+
+bool rangeset_spans(RangeSet *rs, BigNum *first, BigNum *last) {
+    ZigList<Range> cur_list_value = {0};
+    ZigList<Range> other_list_value = {0};
+    ZigList<Range> *cur_list = &cur_list_value;
+    ZigList<Range> *other_list = &other_list_value;
+
+    for (size_t i = 0; i < rs->src_range_list.length; i += 1) {
+        RangeWithSrc *range_with_src = &rs->src_range_list.at(i);
+        Range *range = &range_with_src->range;
+        cur_list->append({range->first, range->last});
+    }
+
+    BigNum one;
+    bignum_init_unsigned(&one, 1);
+
+    bool changes_made = true;
+    while (changes_made) {
+        changes_made = false;
+        for (size_t cur_i = 0; cur_i < cur_list->length; cur_i += 1) {
+            Range *range = &cur_list->at(cur_i);
+            changes_made = add_range(other_list, range, &one) || changes_made;
+        }
+        ZigList<Range> *tmp = cur_list;
+        cur_list = other_list;
+        other_list = tmp;
+        other_list->resize(0);
+    }
+
+    if (cur_list->length != 1)
+        return false;
+    Range *range = &cur_list->at(0);
+    if (bignum_cmp_neq(&range->first, first))
+        return false;
+    if (bignum_cmp_neq(&range->last, last))
+        return false;
+    return true;
+}
src/range_set.hpp
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2017 Andrew Kelley
+ *
+ * This file is part of zig, which is MIT licensed.
+ * See http://opensource.org/licenses/MIT
+ */
+
+#ifndef ZIG_RANGE_SET_HPP
+#define ZIG_RANGE_SET_HPP
+
+#include "all_types.hpp"
+
+struct Range {
+    BigNum first;
+    BigNum last;
+};
+
+struct RangeWithSrc {
+    Range range;
+    AstNode *source_node;
+};
+
+struct RangeSet {
+    ZigList<RangeWithSrc> src_range_list;
+};
+
+AstNode *rangeset_add_range(RangeSet *rs, BigNum *first, BigNum *last, AstNode *source_node);
+bool rangeset_spans(RangeSet *rs, BigNum *first, BigNum *last);
+
+#endif
test/cases/switch.zig
@@ -1,6 +1,6 @@
 const assert = @import("std").debug.assert;
 
-test "switchWithNumbers" {
+test "switch with numbers" {
     testSwitchWithNumbers(13);
 }
 
@@ -13,7 +13,7 @@ fn testSwitchWithNumbers(x: u32) {
     assert(result);
 }
 
-test "switchWithAllRanges" {
+test "switch with all ranges" {
     assert(testSwitchWithAllRanges(50, 3) == 1);
     assert(testSwitchWithAllRanges(101, 0) == 2);
     assert(testSwitchWithAllRanges(300, 5) == 3);
@@ -29,7 +29,7 @@ fn testSwitchWithAllRanges(x: u32, y: u32) -> u32 {
     }
 }
 
-test "implicitComptimeSwitch" {
+test "implicit comptime switch" {
     const x = 3 + 4;
     const result = switch (x) {
         3 => 10,
@@ -44,7 +44,7 @@ test "implicitComptimeSwitch" {
     }
 }
 
-test "switchOnEnum" {
+test "switch on enum" {
     const fruit = Fruit.Orange;
     nonConstSwitchOnEnum(fruit);
 }
@@ -62,7 +62,7 @@ fn nonConstSwitchOnEnum(fruit: Fruit) {
 }
 
 
-test "switchStatement" {
+test "switch statement" {
     nonConstSwitch(SwitchStatmentFoo.C);
 }
 fn nonConstSwitch(foo: SwitchStatmentFoo) {
@@ -82,7 +82,7 @@ const SwitchStatmentFoo = enum {
 };
 
 
-test "switchProngWithVar" {
+test "switch prong with variable" {
     switchProngWithVarFn(SwitchProngWithVarEnum.One {13});
     switchProngWithVarFn(SwitchProngWithVarEnum.Two {13.0});
     switchProngWithVarFn(SwitchProngWithVarEnum.Meh);
@@ -107,7 +107,7 @@ fn switchProngWithVarFn(a: &const SwitchProngWithVarEnum) {
 }
 
 
-test "switchWithMultipleExpressions" {
+test "switch with multiple expressions" {
     const x = switch (returnsFive()) {
         1, 2, 3 => 1,
         4, 5, 6 => 2,
@@ -135,7 +135,7 @@ fn returnsFalse() -> bool {
         Number.Three => |x| return x > 12.34,
     }
 }
-test "switchOnConstEnumWithVar" {
+test "switch on const enum with var" {
     assert(!returnsFalse());
 }
 
@@ -150,3 +150,41 @@ fn trueIfBoolFalseOtherwise(comptime T: type) -> bool {
         else => false,
     }
 }
+
+test "switch handles all cases of number" {
+    testSwitchHandleAllCases();
+    comptime testSwitchHandleAllCases();
+}
+
+const u2 = @IntType(false, 2);
+fn testSwitchHandleAllCases() {
+    assert(testSwitchHandleAllCasesExhaustive(0) == 3);
+    assert(testSwitchHandleAllCasesExhaustive(1) == 2);
+    assert(testSwitchHandleAllCasesExhaustive(2) == 1);
+    assert(testSwitchHandleAllCasesExhaustive(3) == 0);
+
+    assert(testSwitchHandleAllCasesRange(100) == 0);
+    assert(testSwitchHandleAllCasesRange(200) == 1);
+    assert(testSwitchHandleAllCasesRange(201) == 2);
+    assert(testSwitchHandleAllCasesRange(202) == 4);
+    assert(testSwitchHandleAllCasesRange(230) == 3);
+}
+
+fn testSwitchHandleAllCasesExhaustive(x: u2) -> u2 {
+    switch (x) {
+        0 => u2(3),
+        1 => 2,
+        2 => 1,
+        3 => 0,
+    }
+}
+
+fn testSwitchHandleAllCasesRange(x: u8) -> u8 {
+    switch (x) {
+        0 ... 100 => u8(0),
+        101 ... 200 => 1,
+        201, 203 => 2,
+        202 => 4,
+        204 ... 255 => 3,
+    }
+}
test/cases/try.zig
@@ -12,6 +12,7 @@ fn tryOnErrorUnionImpl() {
     } else |err| switch (err) {
         error.ItBroke, error.NoMem => 1,
         error.CrappedOut => i32(2),
+        else => unreachable,
     };
     assert(x == 11);
 }
test/compile_errors.zig
@@ -542,7 +542,46 @@ pub fn addCases(cases: &tests.CompileErrorContext) {
             ".tmp_source.zig:5:5: error: redefinition of 'Bar'",
             ".tmp_source.zig:2:1: note: previous definition is here");
 
-    cases.add("multiple else prongs in a switch",
+    cases.add("switch expression - missing enumeration prong",
+        \\const Number = enum {
+        \\    One,
+        \\    Two,
+        \\    Three,
+        \\    Four,
+        \\};
+        \\fn f(n: Number) -> i32 {
+        \\    switch (n) {
+        \\        Number.One => 1,
+        \\        Number.Two => 2,
+        \\        Number.Three => i32(3),
+        \\    }
+        \\}
+        \\
+        \\export fn entry() -> usize { @sizeOf(@typeOf(f)) }
+    , ".tmp_source.zig:8:5: error: enumeration value 'Number.Four' not handled in switch");
+
+    cases.add("switch expression - duplicate enumeration prong",
+        \\const Number = enum {
+        \\    One,
+        \\    Two,
+        \\    Three,
+        \\    Four,
+        \\};
+        \\fn f(n: Number) -> i32 {
+        \\    switch (n) {
+        \\        Number.One => 1,
+        \\        Number.Two => 2,
+        \\        Number.Three => i32(3),
+        \\        Number.Four => 4,
+        \\        Number.Two => 2,
+        \\    }
+        \\}
+        \\
+        \\export fn entry() -> usize { @sizeOf(@typeOf(f)) }
+    , ".tmp_source.zig:13:15: error: duplicate switch value",
+      ".tmp_source.zig:10:15: note: other value is here");
+
+    cases.add("switch expression - multiple else prongs",
         \\fn f(x: u32) {
         \\    const value: bool = switch (x) {
         \\        1234 => false,
@@ -555,6 +594,41 @@ pub fn addCases(cases: &tests.CompileErrorContext) {
         \\}
     , ".tmp_source.zig:5:9: error: multiple else prongs in switch expression");
 
+    cases.add("switch expression - non exhaustive integer prongs",
+        \\fn foo(x: u8) {
+        \\    switch (x) {
+        \\        0 => {},
+        \\    }
+        \\}
+        \\export fn entry() -> usize { @sizeOf(@typeOf(foo)) }
+    ,
+        ".tmp_source.zig:2:5: error: switch must handle all possibilities");
+
+    cases.add("switch expression - duplicate or overlapping integer value",
+        \\fn foo(x: u8) -> u8 {
+        \\    switch (x) {
+        \\        0 ... 100 => u8(0),
+        \\        101 ... 200 => 1,
+        \\        201, 203 ... 207 => 2,
+        \\        206 ... 255 => 3,
+        \\    }
+        \\}
+        \\export fn entry() -> usize { @sizeOf(@typeOf(foo)) }
+    ,
+        ".tmp_source.zig:6:9: error: duplicate switch value",
+        ".tmp_source.zig:5:14: note: previous value is here");
+
+    cases.add("switch expression - switch on pointer type with no else",
+        \\fn foo(x: &u8) {
+        \\    switch (x) {
+        \\        &y => {},
+        \\    }
+        \\}
+        \\const y: u8 = 100;
+        \\export fn entry() -> usize { @sizeOf(@typeOf(foo)) }
+    ,
+        ".tmp_source.zig:2:5: error: else prong required when switching on type '&u8'");
+
     cases.add("global variable initializer must be constant expression",
         \\extern fn foo() -> i32;
         \\const x = foo();
@@ -716,24 +790,6 @@ pub fn addCases(cases: &tests.CompileErrorContext) {
             ".tmp_source.zig:4:26: error: division by zero is undefined");
 
 
-    cases.add("missing switch prong",
-        \\const Number = enum {
-        \\    One,
-        \\    Two,
-        \\    Three,
-        \\    Four,
-        \\};
-        \\fn f(n: Number) -> i32 {
-        \\    switch (n) {
-        \\        Number.One => 1,
-        \\        Number.Two => 2,
-        \\        Number.Three => i32(3),
-        \\    }
-        \\}
-        \\
-        \\export fn entry() -> usize { @sizeOf(@typeOf(f)) }
-    , ".tmp_source.zig:8:5: error: enumeration value 'Number.Four' not handled in switch");
-
     cases.add("normal string with newline",
         \\const foo = "a
         \\b";
CMakeLists.txt
@@ -56,6 +56,7 @@ set(ZIG_SOURCES
     "${CMAKE_SOURCE_DIR}/src/main.cpp"
     "${CMAKE_SOURCE_DIR}/src/os.cpp"
     "${CMAKE_SOURCE_DIR}/src/parser.cpp"
+    "${CMAKE_SOURCE_DIR}/src/range_set.cpp"
     "${CMAKE_SOURCE_DIR}/src/target.cpp"
     "${CMAKE_SOURCE_DIR}/src/tokenizer.cpp"
     "${CMAKE_SOURCE_DIR}/src/util.cpp"