Welcome to the Tsonnet series!
If you're not following along, check out how it all started in the first post of the series.
In the previous post, we got computed field names working:
Turns out the catch-all patterns I'd been relying on were defeating the one thing that makes pattern-matching useful.
The problem
Compilers are amazing co-pilots when you're working with strongly typed languages. They catch invariants when we forget. But that only works if we enforce checking all pattern-matching cases, one by one. A catch-all | _ -> or | expr -> is the compiler equivalent of duct tape — it shuts it up, but it also hides every variant you forgot to handle.
So far, I'd been mistakenly ignoring new type variants in pattern-matching across lib/scope.ml, lib/type.ml, and lib/interpreter.ml because of catch-all cases. Every time a new expr variant was added, the compiler stayed quiet.
The fix: explicit exhaustiveness
The first thing to do is remove the catch-all cases and list every variant explicitly. This way, when a new variant gets added to the AST, OCaml will refuse to compile until every match is updated.
lib/scope.ml — _validate
diff --git a/lib/scope.ml b/lib/scope.ml
index 3f329fe..48dfa83 100644
--- a/lib/scope.ml
+++ b/lib/scope.ml
@@ -31,7 +31,6 @@ let add_locals_to_context locals context = {
let rec _validate expr context =
match expr with
- | Unit | Null _ | Number _ | String _ | Bool _ -> ok ()
| Ident (pos, varname) ->
(* Identifier validation - the heart of scope checking *)
validate_ident pos varname context
@@ -52,9 +51,10 @@ let rec _validate expr context =
_validate expr context
| IndexedExpr (_, _, index_expr) ->
_validate index_expr context
- | _ ->
- (* For any other expression types, no special scope validation needed *)
- ok ()
+ (* For any other expression types, no special scope validation needed *)
+ | Unit | Null _ | Number _ | String _ | Bool _ | EvaluatedObject _
+ | RuntimeObject _ | ObjectPtr _ | FunctionDef _ | FunctionCall _ | Closure _
+ | If _ -> ok ()
lib/type.ml — three matches
The type checker had the most catch-all cases. Three separate matches needed the treatment:
diff --git a/lib/type.ml b/lib/type.ml
index 79fa6aa..28c8909 100644
--- a/lib/type.ml
+++ b/lib/type.ml
@@ -100,7 +100,6 @@ let rec to_string = function
| Tunresolved -> "<unresolved>"
let rec collect_free_idents = function
- | Unit | Null _ | Number _ | String _ | Bool _ -> []
| Ident (_, name) -> [name]
| Array (_, exprs) -> List.concat_map collect_free_idents exprs
| BinOp (_, _, e1, e2) -> collect_free_idents e1 @ collect_free_idents e2
@@ -126,7 +125,9 @@ let rec collect_free_idents = function
| Named (_, e) -> collect_free_idents e
) call.args
| Closure (_, closure) -> collect_free_idents closure.body
- | _ -> []
+ | Unit | Null _ | Number _ | String _ | Bool _ | EvaluatedObject _
+ | RuntimeObject _ | ObjectPtr _ | FunctionDef _
+ | If _ -> []
@@ -154,7 +155,6 @@ let rec check_cyclic_refs venv varname seen pos =
| _ -> ok ()
and check_expr_for_cycles venv expr seen =
match expr with
- | Unit | Null _ | Number _ | String _ | Bool _ -> ok ()
| Array (_, exprs) -> iter_for_cycles venv seen exprs
| ParsedObject (_, entries) -> check_object_for_cycles venv entries seen
| ObjectFieldAccess (pos, scope, exprs) -> check_object_field_chain_for_cycles venv (pos, scope, exprs) seen
@@ -163,7 +163,10 @@ and check_expr_for_cycles venv expr seen =
| UnaryOp (_, _, e) -> check_expr_for_cycles venv e seen
| Seq exprs -> iter_for_cycles venv seen exprs
| If (_, cond_expr, then_expr, else_expr_opt) -> check_conditional_for_cycles venv (cond_expr, then_expr, else_expr_opt) seen
- | _ -> ok ()
+ | Unit | Null _ | Number _ | String _ | Bool _ | EvaluatedObject _
+ | RuntimeObject _ | ObjectPtr _ | FunctionDef _ | FunctionCall _ | Closure _
+ | Local _ | IndexedExpr _ -> ok ()
@@ -235,7 +238,7 @@ let rec translate venv expr =
| Closure (pos, closure) -> translate_closure venv (pos, closure)
| If (pos, cond_expr, then_expr, else_expr_opt) ->
translate_conditional venv (pos, cond_expr, then_expr, else_expr_opt)
- | expr' ->
+ | EvaluatedObject _ | RuntimeObject _ | ObjectPtr _ as expr' ->
error (Error.Msg.type_invalid_expr (string_of_type expr'))
lib/interpreter.ml — deep_eval
diff --git a/lib/interpreter.ml b/lib/interpreter.ml
index 40b9eea..1751554 100644
--- a/lib/interpreter.ml
+++ b/lib/interpreter.ml
@@ -562,7 +562,9 @@ let rec deep_eval expr =
| RuntimeObject _ ->
let* (_, evaluated) = interpret Env.empty expr in
deep_eval evaluated
- | expr ->
+ | Ident _ | ParsedObject _ | ObjectPtr _ | ObjectFieldAccess _
+ | BinOp _ | UnaryOp _ | IndexedExpr _ | Local _ | Seq _
+ | FunctionDef _ | FunctionCall _ | Closure _ | If _ as expr ->
let* (_, evaluated) = interpret Env.empty expr in
deep_eval evaluated
Every catch-all is gone. Now when I add a new variant to the expr type, the compiler will tell me exactly which function needs a new arm — silently swallowing it is no longer an option.
Does it change anything? (No)
This improves nothing in the runtime behaviour. Every explicit list above matches exactly what the catch-all was doing. But now the compiler can do its job — if I forget to update a match when adding a new variant, the build breaks.
The existing cram tests pass without changes. No output changed — as expected, since the semantics are identical.
What's next
This post was just the tip of the iceberg — removing the catch-all exposed the loose ends, but now I actually need to deal with them. In the next one, I'll start with the scope check, where there are variants in _validate that are silently passing through when they probably shouldn't. But first, I needed to see them.
The diff is small — the habit it breaks is the point.
The entire diff can be seen here.
Thanks for reading Bit Maybe Wise! Your inbox deserves exhaustive coverage — subscribe and never miss a variant.
Photo by Volodymyr Hryshchenko on Unsplash
Top comments (0)