Enums in a switch can now be required to be exhaustive without a default

dev/src/lobster/codegen.h

@@ -1696,8 +1696,7 @@ void Switch::Generate(CodeGen &cg, size_t retval) const {
         cg.SetLabels(thiscase);
         cg.GenPop(valtlt);
         cg.TakeTemp(1, false);
-        cg.Gen(cas->cbody, retval);
-        if (retval) cg.TakeTemp(1, true);
+        cas->Generate(cg, retval);
         bs.End();
         if (n != cases->children.back() || !have_default) {
             cg.EmitOp(IL_JUMP);
@@ -1784,8 +1783,7 @@ void Switch::GenerateJumpTableMain(CodeGen & cg, size_t retval, int range, int m
         }
         if (cas->pattern->children.empty()) default_pos = cg.Pos();
         cg.EmitOp(IL_JUMP_TABLE_CASE_START);
-        cg.Gen(cas->cbody, retval);
-        if (retval) cg.TakeTemp(1, true);
+        cas->Generate(cg, retval);
         bs.End();
         if (n != cases->children.back()) {
             cg.EmitOp(IL_JUMP);
@@ -1817,8 +1815,17 @@ void Switch::GenerateTypeDispatch(CodeGen &cg, size_t retval) const {
     GenerateJumpTableMain(cg, retval, range, 0);
 }
 
-void Case::Generate(CodeGen &/*cg*/, size_t /*retval*/) const {
-    assert(false);
+void Case::Generate(CodeGen &cg, size_t retval) const {
+    if (cbody->Arity()) {
+        cg.Gen(cbody, retval);
+        if (retval) cg.TakeTemp(1, true);
+    } else {
+        // An empty default case signals runtime error for enums.
+        assert(pattern->children.empty());
+        // FIXME: would be great to ensure the offending value is still on the stack for
+        // this instruction to have access to.
+        cg.EmitOp(IL_ENUM_RANGE_ERR);
+    }
 }
 
 void Range::Generate(CodeGen &/*cg*/, size_t /*retval*/) const {

dev/src/lobster/il.h

@@ -146,7 +146,8 @@ enum MathOp {
     F(LV_FVSMOD, 1, 1, 0) \
     F(LV_SADD, 0, 1, 0) \
     F(LV_IPP, 0, 0, 0) F(LV_IMM, 0, 0, 0) \
-    F(LV_FPP, 0, 0, 0) F(LV_FMM, 0, 0, 0)
+    F(LV_FPP, 0, 0, 0) F(LV_FMM, 0, 0, 0) \
+    F(ENUM_RANGE_ERR, 0, 0, 0)
 
 #define ILCALLNAMES \
     F(PUSHFUN, 1, 0, 1)

dev/src/lobster/typecheck.h

@@ -2698,15 +2698,26 @@ Node *Switch::TypeCheck(TypeChecker &tc, size_t reqret) {
         de->is_switch_dispatch = true;
         de->is_dispatch_root = true;
         de->subudts_size = dispatch_udt.subudts.size();
-    } else {
-        if (default_loc < 0) {
-            if (reqret) tc.Error(*this, "switch that returns a value must have a default case");
-            if (ptype->IsEnum()) {
-                for (auto [i, ev] : enumerate(ptype->e->vals)) {
-                    if (!enum_cases[i])
-                        tc.Error(*value, "enum value ", Q(ev->name), " not tested in switch");
-                }
-            }
+    } else if (default_loc < 0) {
+        if (ptype->IsEnum()) {
+            for (auto [i, ev] : enumerate(ptype->e->vals)) {
+                if (!enum_cases[i])
+                    tc.Error(*value, "enum value ", Q(ev->name), " not tested in switch");
+            }
+            // Add a runtime error for when the value is out of range.
+            auto pat = new List(cases->line);
+            pat->exptype = type_void;
+            pat->lt = LT_ANY;
+            // Blocks always have minimum of 1 statement in them, so an empty one signals runtime error here.
+            auto blk = new Block(cases->line);
+            blk->exptype = type_void;
+            blk->lt = LT_ANY;
+            auto cas = new Case(cases->line, pat, blk);
+            cas->exptype = type_void;
+            cas->lt = LT_ANY;
+            cases->Add(cas);
+        } else {
+            if (reqret) tc.Error(*this, "non-exhaustive switch that returns a value must have a default case");
         }
     }
     lt = LT_KEEP;
@@ -3890,7 +3901,10 @@ bool Switch::Terminal(TypeChecker &tc) const {
         if (cas->pattern->children.empty()) have_default = true;
         if (!cas->cbody->Terminal(tc)) return false;
     }
-    // FIXME: this should return true if the switch is proven exhaustive for types, sadly cannot guarantee that with enums.
+    if (!value->exptype.Null() &&  // Should already been typechecked but just in case.
+        (value->exptype->t == V_CLASS || value->exptype->IsEnum()))  // Guaranteed exhaustive or runtime error.
+        return true;
+    // Other types, cannot guarantee it is terminal without a default.
     return have_default;
 }
 

dev/src/lobster/vmops.h

@@ -797,6 +797,10 @@ VM_INLINE void U_ABORT(VM &vm, StackPtr) {
     vm.SeriousError("VM internal error: abort");
 }
 
+VM_INLINE void U_ENUM_RANGE_ERR(VM &vm, StackPtr) {
+    vm.Error("Enum out of range of possible values in switch");
+}
+
 VM_INLINE void U_LVAL_VARL(VM &, StackPtr, int) {
     assert(false);
 }

docs/language_reference.html

@@ -995,10 +995,14 @@ <h2 id="control-structures">Control Structures</h2>
             &quot;yes{x}&quot;
         case 4..6, 8: &quot;maybe&quot;
         default: &quot;what?&quot;</code></pre>
-<p>The value you switch on may be int, float, string or class instance.
-Cases may test for multiple values, even ranges (which are inclusive).
-When testing for class instances, the cases are types which must
-exhaustively cover all non-abstract sub-classes of the class type.</p>
+<p>The value you switch on may be int, enum, float, string or class
+instance. Cases may test for multiple values, even ranges (which are
+inclusive). When testing for class instances, the cases are types which
+must exhaustively cover all non-abstract sub-classes of the class type.
+Enums are also required to be tested exhaustively. An enum value that
+doesn't correspond to a value in the current enum definition (such as
+one read from a file) will produce a runtime error if the switch does
+not have a default case.</p>
 <p><code>guard</code> is a special variant of <code>if</code> for
 writing code in "early-out" style:</p>
 <pre><code>guard a &gt;= 0:

docs/source/language_reference.md

@@ -1183,9 +1183,12 @@ var st = switch i:
         default: "what?"
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The value you switch on may be int, float, string or class instance. Cases may test for multiple
+The value you switch on may be int, enum, float, string or class instance. Cases may test for multiple
 values, even ranges (which are inclusive). When testing for class instances, the cases are
 types which must exhaustively cover all non-abstract sub-classes of the class type.
+Enums are also required to be tested exhaustively. An enum value that doesn't correspond
+to a value in the current enum definition (such as one read from a file) will produce a runtime
+error if the switch does not have a default case.
 
 `guard` is a special variant of `if` for writing code in "early-out" style:
 

samples/will_it_shuffle.lobster

@@ -31,7 +31,6 @@ def mode_name(): return switch mode:
     case shuffle_naive_swap: "naive swap"
     case shuffle_push_remove: "push remove"
     case shuffle_fisher_yates: "fisher yates"
-    default: ""
 
 def naive_swap(xs):
     // ala https://blog.codinghorror.com/the-danger-of-naivete/

tests/typeswitch.lobster

@@ -75,3 +75,36 @@ run_test("typeswitch"):
             case F: t.f
 
     assert equal(results4, [ 0, 0, 0, 5 ])
+
+    // Similarly, enums need to be checked exhaustively. A compile time error will result
+    // if any enum value is not covered.
+
+    let btests = [ true, false /*, bool(2)*/ ]
+
+    let results5 = map(btests) t:
+        switch t:
+            case true: "t"
+            case false: "f"
+            // No default needed!
+            // If bool(2) is a possible value, this switch will generate a runtime error.
+
+    assert equal(results5, [ "t", "f" ])
+
+    let btests2 = [ true, false, bool(2) ]
+
+    let results6 = map(btests2) t:
+        switch t:
+            case true: "t"
+            case false: "f"
+            default:
+                // Unlike class instances which can't be forged, it is possible to cast any
+                // integer to an enum. This must generally be possible to load files that may
+                // have old/incorrect enum values stored.
+                // If this is possible, a default case should be used, since in the switch above
+                // this one, a lack of a default case will result in a runtime error if an
+                // illegal value is used.
+                // Of course, a default case has the disadvantage that you will not get errors
+                // about new enum values not having their own case.
+                "?"
+
+    assert equal(results6, [ "t", "f", "?" ])