diff --git a/dev/src/lobster/codegen.h b/dev/src/lobster/codegen.h index a4eaeae6..173aa1d0 100644 --- a/dev/src/lobster/codegen.h +++ b/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 { diff --git a/dev/src/lobster/il.h b/dev/src/lobster/il.h index fc2b8bad..d16c522b 100644 --- a/dev/src/lobster/il.h +++ b/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) diff --git a/dev/src/lobster/typecheck.h b/dev/src/lobster/typecheck.h index 01df6469..d24a4d21 100644 --- a/dev/src/lobster/typecheck.h +++ b/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; } diff --git a/dev/src/lobster/vmops.h b/dev/src/lobster/vmops.h index 04f78a3b..49534245 100644 --- a/dev/src/lobster/vmops.h +++ b/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); } diff --git a/docs/language_reference.html b/docs/language_reference.html index 447eacc0..0d144324 100644 --- a/docs/language_reference.html +++ b/docs/language_reference.html @@ -995,10 +995,14 @@
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.
+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:
guard a >= 0:
diff --git a/docs/source/language_reference.md b/docs/source/language_reference.md
index d2c94f3e..586c18b8 100644
--- a/docs/source/language_reference.md
+++ b/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:
diff --git a/samples/will_it_shuffle.lobster b/samples/will_it_shuffle.lobster
index cf5bd989..a42c2fbf 100644
--- a/samples/will_it_shuffle.lobster
+++ b/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/
diff --git a/tests/typeswitch.lobster b/tests/typeswitch.lobster
index 0b13bf1c..3afc74b0 100644
--- a/tests/typeswitch.lobster
+++ b/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", "?" ])