From 9f48661fb7ace56a1726d09076d62a96da789512 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Thu, 5 Feb 2026 11:56:13 +0000 Subject: [PATCH 1/5] Add disabled repro test for #988 Underscore-prefixed variables in overlapping patterns cause segfaults and incorrect results due to a binding collision in match lowering. Co-Authored-By: Claude Opus 4.5 --- .../todo/under_ident_pattern.exp.disabled | 1 + .../tests/todo/under_ident_pattern.failing | 0 .../tests/todo/under_ident_pattern.sk.disabled | 18 ++++++++++++++++++ 3 files changed, 19 insertions(+) create mode 100644 skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled create mode 100644 skiplang/compiler/tests/todo/under_ident_pattern.failing create mode 100644 skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled b/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled new file mode 100644 index 000000000..00750edc0 --- /dev/null +++ b/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled @@ -0,0 +1 @@ +3 diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.failing b/skiplang/compiler/tests/todo/under_ident_pattern.failing new file mode 100644 index 000000000..e69de29bb diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled b/skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled new file mode 100644 index 000000000..c96a0b13c --- /dev/null +++ b/skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled @@ -0,0 +1,18 @@ +base class T { + children = + | T1(x: Int, y: Int, z: Int) + | T2(x: Int, y: Int, z: String) + + fun broken(): Int { + this match { + | T1(_x, _y, _) + | T2(_x, _y, _) -> + _x + _y + } + } +} + +fun main(): void { + t2 = T2(1, 2, "three"); + print_string(t2.broken().toString()) +} From a07fd5cc8eb61d3e87f71317c35751a6497762d3 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Fri, 6 Feb 2026 16:08:52 +0000 Subject: [PATCH 2/5] Use !x instead of __x in ? operator desugaring The ? operator desugars to a match expression that binds the inner value to a temporary variable. This variable was named `__x` which starts with underscore. Rename to `!x` to follow the compiler temp variable convention and avoid triggering the underscore-prefixed variable restriction. Co-Authored-By: Claude Opus 4.5 --- skiplang/compiler/src/convertTree.sk | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/skiplang/compiler/src/convertTree.sk b/skiplang/compiler/src/convertTree.sk index 68f26fa4e..76bf17d17 100644 --- a/skiplang/compiler/src/convertTree.sk +++ b/skiplang/compiler/src/convertTree.sk @@ -1383,13 +1383,13 @@ class Converter{file: FileCache.InputSource} { range, SkipAst.Pat_type( SkipAst.Tid_object(SkipAst.Tclass((range, "Success"))), - Some(Positional(Array[(range, SkipAst.Pat_var((range, "__x")))])), + Some(Positional(Array[(range, SkipAst.Pat_var((range, "!x")))])), SkipAst.Complete(), ), ), ], None(), - (range, SkipAst.Var((range, "__x"))), + (range, SkipAst.Var((range, "!x"))), ); early_return_branch = ( List[ @@ -1397,13 +1397,13 @@ class Converter{file: FileCache.InputSource} { range, SkipAst.Pat_type( SkipAst.Tid_object(SkipAst.Tclass((range, "Failure"))), - Some(Positional(Array[(range, SkipAst.Pat_var((range, "__x")))])), + Some(Positional(Array[(range, SkipAst.Pat_var((range, "!x")))])), SkipAst.Complete(), ), ), ], None(), - (range, SkipAst.Return((range, SkipAst.Var((range, "__x"))))), + (range, SkipAst.Return((range, SkipAst.Var((range, "!x"))))), ); SkipAst.Match( From d4baed68ed2b67bce974b178e62d2882c9bd7f3b Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Fri, 6 Feb 2026 16:10:11 +0000 Subject: [PATCH 3/5] Use explicit binding for underscore-prefixed fields in patterns When pattern matching on fields with underscore prefix (e.g., _value_name), use explicit binding syntax `_value_name => value_name` to bind the value to a non-underscore variable name. This allows the value to be used while avoiding the underscore-prefixed variable restriction. Co-Authored-By: Claude Opus 4.5 --- skiplang/cli/src/usage.sk | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/skiplang/cli/src/usage.sk b/skiplang/cli/src/usage.sk index d43e46991..cd971c1c0 100644 --- a/skiplang/cli/src/usage.sk +++ b/skiplang/cli/src/usage.sk @@ -85,11 +85,11 @@ private fun usageOptions(options: Sequence): String { | _ -> ` --${arg.name}` }; optFlags = arg match { - | Cli.ValuedArg{_value_name} -> + | Cli.ValuedArg{_value_name => value_name} -> if (arg._repeatable) { - `${baseOptFlags} [<${_value_name}>]` + `${baseOptFlags} [<${value_name}>]` } else { - `${baseOptFlags} <${_value_name}>` + `${baseOptFlags} <${value_name}>` } | _ if (arg._repeatable) -> `${baseOptFlags}...` | _ -> baseOptFlags @@ -144,8 +144,11 @@ fun usage( values = cmd._args.filterMap( (arg -> (arg match { - | ValuedArg{_value_name, _value_about} if (_value_about.size() > 0) -> - Some((_value_name, _value_about)) + | ValuedArg{ + _value_name => value_name, + _value_about => value_about, + } if (value_about.size() > 0) -> + Some((value_name, value_about)) | _ -> None() })), ); From 6cd920097ae42c7def23276dacb6b86c194bd227 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Fri, 6 Feb 2026 16:18:18 +0000 Subject: [PATCH 4/5] Forbid underscore-prefixed variable values in patterns Don't bind underscore-prefixed variables (_x, _y, etc.) in pattern matching. They are still allowed in patterns but act as wildcards, so reading their value triggers the existing "You cannot use a variable that starts with _" error. This fixes issue #988 where overlapping patterns with _x-style variables caused binding collisions in the lowering phase, resulting in segfaults or incorrect results. Changes: - check_locals_pattern: Skip binding for is_wildcard_name variables - bind_pattern_name: Use startsWith("_") for duplicate checking - Enable the repro test from tests/todo/ in tests/syntax/invalid/ Co-Authored-By: Claude Opus 4.5 --- skiplang/compiler/src/skipNaming.sk | 8 ++++---- skiplang/compiler/tests/syntax/invalid/under_ident_3.exp | 1 + .../compiler/tests/syntax/invalid/under_ident_3.exp_err | 7 +++++++ .../invalid/under_ident_3.sk} | 0 .../compiler/tests/todo/under_ident_pattern.exp.disabled | 1 - skiplang/compiler/tests/todo/under_ident_pattern.failing | 0 6 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 skiplang/compiler/tests/syntax/invalid/under_ident_3.exp create mode 100644 skiplang/compiler/tests/syntax/invalid/under_ident_3.exp_err rename skiplang/compiler/tests/{todo/under_ident_pattern.sk.disabled => syntax/invalid/under_ident_3.sk} (100%) delete mode 100644 skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled delete mode 100644 skiplang/compiler/tests/todo/under_ident_pattern.failing diff --git a/skiplang/compiler/src/skipNaming.sk b/skiplang/compiler/src/skipNaming.sk index 5b3b1f4cc..40072cbf4 100644 --- a/skiplang/compiler/src/skipNaming.sk +++ b/skiplang/compiler/src/skipNaming.sk @@ -439,9 +439,8 @@ fun pattern_bindings(acc: UMap, pat: A.Pattern): UMap { } } -// TODO consider !name.i1.startsWith("_") fun bind_pattern_name(acc: UMap, name: A.Name): UMap { - if (name.i1 != "_") acc.add(name, void) else acc + if (!name.i1.startsWith("_")) acc.add(name, void) else acc } fun bind_pattern_params( @@ -940,14 +939,15 @@ fun check_locals_pattern(bound: Bound, pat: N.Pattern): Bound { | N.Pat_const _ | N.Pat_literal _ -> bound - | N.Pat_var(var_name) -> bound.bind(var_name) + | N.Pat_var(var_name) -> + if (is_wildcard_name(var_name)) bound else bound.bind(var_name) | N.Pat_type(_, obj_params, _) -> obj_params match { | None() -> bound | Some(params) -> params.foldl(check_locals_pattern, bound) } | N.Pat_as(p, var_name) -> - !bound = bound.bind(var_name); + !bound = if (is_wildcard_name(var_name)) bound else bound.bind(var_name); check_locals_pattern(bound, p) } } diff --git a/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp b/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp @@ -0,0 +1 @@ + diff --git a/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp_err b/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp_err new file mode 100644 index 000000000..7bda70834 --- /dev/null +++ b/skiplang/compiler/tests/syntax/invalid/under_ident_3.exp_err @@ -0,0 +1,7 @@ +File "tests/syntax/invalid/under_ident_3.sk", line 10, characters 7-8: +You cannot use a variable that starts with _ + 8 | | T1(_x, _y, _) + 9 | | T2(_x, _y, _) -> +10 | _x + _y + | ^^ +11 | } diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled b/skiplang/compiler/tests/syntax/invalid/under_ident_3.sk similarity index 100% rename from skiplang/compiler/tests/todo/under_ident_pattern.sk.disabled rename to skiplang/compiler/tests/syntax/invalid/under_ident_3.sk diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled b/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled deleted file mode 100644 index 00750edc0..000000000 --- a/skiplang/compiler/tests/todo/under_ident_pattern.exp.disabled +++ /dev/null @@ -1 +0,0 @@ -3 diff --git a/skiplang/compiler/tests/todo/under_ident_pattern.failing b/skiplang/compiler/tests/todo/under_ident_pattern.failing deleted file mode 100644 index e69de29bb..000000000 From b2f07e3205b3a412b152f2b8ea9e4131fab84560 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Fri, 6 Feb 2026 16:18:42 +0000 Subject: [PATCH 5/5] Treat _-prefixed bindings as underscores in lowering Defense in depth: when lowering match expressions, treat any binding whose name starts with _ as an underscore binding (not just exact "_"). This prevents creating bindings for _x-style variables during match lowering, complementing the fix in skipNaming.sk. Co-Authored-By: Claude Opus 4.5 --- skiplang/compiler/src/skipOuterIstUtils.sk | 2 +- .../compiler/tests/syntax/invalid/under_ident_7.exp_err | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/skiplang/compiler/src/skipOuterIstUtils.sk b/skiplang/compiler/src/skipOuterIstUtils.sk index aaee40160..964fad598 100644 --- a/skiplang/compiler/src/skipOuterIstUtils.sk +++ b/skiplang/compiler/src/skipOuterIstUtils.sk @@ -1544,7 +1544,7 @@ fun binding_needs_ref(binding: O.BindingInfo): Bool { } fun is_underscore_binding(b: O.Binding): Bool { - b.name.id == "_" + b.name.id.startsWith("_") } /* parameters */ diff --git a/skiplang/compiler/tests/syntax/invalid/under_ident_7.exp_err b/skiplang/compiler/tests/syntax/invalid/under_ident_7.exp_err index d3b977805..95ec411b3 100644 --- a/skiplang/compiler/tests/syntax/invalid/under_ident_7.exp_err +++ b/skiplang/compiler/tests/syntax/invalid/under_ident_7.exp_err @@ -1,7 +1,7 @@ -File "tests/syntax/invalid/under_ident_7.sk", line 4, characters 11-11: -Unbound name: _ +File "tests/syntax/invalid/under_ident_7.sk", line 4, characters 22-22: +You cannot use a variable that starts with _ 2 | fun matcher(): T { 3 | this match { 4 | | Foo{_ => _} -> _ - | ^ + | ^ 5 | }