Commit db988f42a7

LemonBoy <thatlemon@gmail.com>
2019-09-23 11:14:36
Fix computation of switch coverage
Closes #3258
1 parent 53ae03e
Changed files (3)
src/range_set.cpp
@@ -4,8 +4,12 @@ AstNode *rangeset_add_range(RangeSet *rs, BigInt *first, BigInt *last, AstNode *
     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 ((bigint_cmp(first, &range->first) != CmpLT && bigint_cmp(first, &range->last) != CmpGT) ||
-            (bigint_cmp(last, &range->first) != CmpLT && bigint_cmp(last, &range->last) != CmpGT))
+        if ((bigint_cmp(first, &range->first) == CmpLT && bigint_cmp(last, &range->first) == CmpLT) ||
+            (bigint_cmp(first, &range->last) == CmpGT && bigint_cmp(last, &range->last) == CmpGT))
+        {
+            // first...last is completely before/after `range`
+        }
+        else
         {
             return range_with_src->source_node;
         }
@@ -16,64 +20,52 @@ AstNode *rangeset_add_range(RangeSet *rs, BigInt *first, BigInt *last, AstNode *
 
 }
 
-static bool add_range(ZigList<Range> *list, Range *new_range, BigInt *one) {
-    for (size_t i = 0; i < list->length; i += 1) {
-        Range *range = &list->at(i);
-
-        BigInt first_minus_one;
-        bigint_sub(&first_minus_one, &range->first, one);
-
-        if (bigint_cmp(&new_range->last, &first_minus_one) == CmpEQ) {
-            range->first = new_range->first;
-            return true;
-        }
-
-        BigInt last_plus_one;
-        bigint_add(&last_plus_one, &range->last, one);
+static int compare_rangeset(const void *a, const void *b) {
+    const Range *r1 = &static_cast<const RangeWithSrc*>(a)->range;
+    const Range *r2 = &static_cast<const RangeWithSrc*>(b)->range;
+    // Assume no two ranges overlap
+    switch (bigint_cmp(&r1->first, &r2->first)) {
+        case CmpLT: return -1;
+        case CmpGT: return  1;
+        case CmpEQ: return  0;
+    }
+    zig_unreachable();
+}
 
-        if (bigint_cmp(&new_range->first, &last_plus_one) == CmpEQ) {
-            range->last = new_range->last;
-            return true;
-        }
+void rangeset_sort(RangeSet *rs) {
+    if (rs->src_range_list.length > 1) {
+        qsort(rs->src_range_list.items, rs->src_range_list.length,
+              sizeof(RangeWithSrc), compare_rangeset);
     }
-    list->append({new_range->first, new_range->last});
-    return false;
 }
 
 bool rangeset_spans(RangeSet *rs, BigInt *first, BigInt *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;
+    rangeset_sort(rs);
 
-    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});
-    }
+    const Range *first_range = &rs->src_range_list.at(0).range;
+    if (bigint_cmp(&first_range->first, first) != CmpEQ)
+        return false;
+
+    const Range *last_range = &rs->src_range_list.last().range;
+    if (bigint_cmp(&last_range->last, last) != CmpEQ)
+        return false;
 
     BigInt one;
     bigint_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);
+    // Make sure there are no holes in the first...last range
+    for (size_t i = 1; i < rs->src_range_list.length; i++) {
+        const Range *range = &rs->src_range_list.at(i).range;
+        const Range *prev_range = &rs->src_range_list.at(i - 1).range;
+
+        assert(bigint_cmp(&prev_range->last, &range->first) == CmpLT);
+
+        BigInt last_plus_one;
+        bigint_add(&last_plus_one, &prev_range->last, &one);
+
+        if (bigint_cmp(&last_plus_one, &range->first) != CmpEQ)
+            return false;
     }
 
-    if (cur_list->length != 1)
-        return false;
-    Range *range = &cur_list->at(0);
-    if (bigint_cmp(&range->first, first) != CmpEQ)
-        return false;
-    if (bigint_cmp(&range->last, last) != CmpEQ)
-        return false;
     return true;
 }
test/stage1/behavior/switch.zig
@@ -425,3 +425,12 @@ test "switch prongs with cases with identical payload types" {
     S.doTheTest();
     comptime S.doTheTest();
 }
+
+test "switch with disjoint range" {
+    var q: u8 = 0;
+    switch (q) {
+        0...125 => {},
+        127...255 => {},
+        126...126 => {},
+    }
+}
test/compile_errors.zig
@@ -2,6 +2,19 @@ const tests = @import("tests.zig");
 const builtin = @import("builtin");
 
 pub fn addCases(cases: *tests.CompileErrorContext) void {
+    cases.add(
+        "switch with overlapping case ranges",
+        \\export fn entry() void {
+        \\    var q: u8 = 0;
+        \\    switch (q) {
+        \\        1...2 => {},
+        \\        0...255 => {},
+        \\    }
+        \\}
+    ,
+        "tmp.zig:5:9: error: duplicate switch value",
+    );
+
     cases.add(
         "attempt to negate a non-integer, non-float or non-vector type",
         \\fn foo() anyerror!u32 {