From 679a9f4d66ec8891ab1ff0a9b205bb2c5cdd367a Mon Sep 17 00:00:00 2001 From: Benjamin Dasnois Date: Fri, 1 May 2026 17:51:37 +0200 Subject: [PATCH] Switch on String: forward `default` into each length bucket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compileSwitchOptimizedForStrings` partitions cases by `String.length` and emits a `switch(__temp.size()) { case N: { /* if-cascade */ } }`, calling `compileSwitchAsIfs` per bucket with `null` for the default expression. The user's `default:` is then only attached to the outer size-switch (`compileDefaultCase(edef)`), which fires when the input's length matches no bucket — but not when the length matches a bucket and the value matches no case inside it. That input falls through the inner if-cascade with no statement executed, silently breaking statement-position string switches and tripping `-Werror=return-type` on value-position ones. Pass `edef` into the bucketed `compileSwitchAsIfs` so each cascade ends with `else { /* user default */ }`. The outer-default emission stays — it still handles unmatched-length inputs. Repro: a switch on a String value with two same-length cases ("A", "B") and a default; pass any other length-1 string and the default never runs. Added two regression tests (statement-position and value-position) under `test/unit_testing/tests/Switch/`. --- src/cxxcompiler/subcompilers/Expressions.hx | 10 +++- test/unit_testing/tests/Switch/Main.hx | 35 ++++++++++++ .../tests/Switch/intended/include/Main.h | 2 + .../tests/Switch/intended/src/Main.cpp | 53 +++++++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) diff --git a/src/cxxcompiler/subcompilers/Expressions.hx b/src/cxxcompiler/subcompilers/Expressions.hx index 2fee1263..ef434801 100644 --- a/src/cxxcompiler/subcompilers/Expressions.hx +++ b/src/cxxcompiler/subcompilers/Expressions.hx @@ -1861,7 +1861,15 @@ class Expressions extends SubCompiler { result += "switch(" + generateCppForStringLength("__temp") + ") {"; for(length => lengthCases in lengths) { result += "\n\tcase " + length + ": {\n"; - result += compileSwitchAsIfs("__temp", eType, lengthCases, null, false).tab(2); + // Pass `edef` so each length-bucket's if-cascade ends with + // the user's default branch. Without this an input whose + // length matches a bucket but whose value matches no case + // inside it falls through with no statement executed — + // silently breaking `switch (s) { case "A": …; default: … }` + // when `s` is, say, "B" (same length as the case but not + // in the bucket), and tripping `-Werror=return-type` when + // the switch is a value position. + result += compileSwitchAsIfs("__temp", eType, lengthCases, edef, false).tab(2); result += "\n\t\tbreak;"; result += "\n\t}"; } diff --git a/test/unit_testing/tests/Switch/Main.hx b/test/unit_testing/tests/Switch/Main.hx index fb77a233..24bbc3f0 100644 --- a/test/unit_testing/tests/Switch/Main.hx +++ b/test/unit_testing/tests/Switch/Main.hx @@ -7,6 +7,29 @@ function assert(b: Bool) { } } +// Statement-position string switch with a default. The bucket for +// length 1 contains both "A" and "B"; any other length-1 input +// must hit the default. +function stmtSwitch(s: String): String { + var matched = "?"; + switch(s) { + case "A": matched = "A"; + case "B": matched = "B"; + default: matched = "default"; + } + return matched; +} + +// Value-position string switch — every path must yield an `Int`, +// otherwise the codegen trips `-Werror=return-type`. +function valueSwitch(s: String): Int { + return switch(s) { + case "A": 1; + case "B": 2; + case _: 99; + } +} + enum Test { One; Two; @@ -38,6 +61,18 @@ function main() { case "Blablabla": assert(false); } + // String switch with same-length cases + default: exercises the + // `compileSwitchOptimizedForStrings` length-bucketed path. Inputs + // matching a bucket's length but no case inside it must fall + // through to `default`. Wrapped in helper functions so each + // emits its own `__temp` (the codegen declares the temp at + // function scope; calling one switch then another from `main` + // would collide). + assert(stmtSwitch("C") == "default"); + assert(stmtSwitch("A") == "A"); + assert(valueSwitch("C") == 99); + assert(valueSwitch("B") == 2); + // --- final result = switch(a) { diff --git a/test/unit_testing/tests/Switch/intended/include/Main.h b/test/unit_testing/tests/Switch/intended/include/Main.h index 5f1add83..6baa2566 100644 --- a/test/unit_testing/tests/Switch/intended/include/Main.h +++ b/test/unit_testing/tests/Switch/intended/include/Main.h @@ -67,6 +67,8 @@ class Main_Fields_ { static int returnCode; static void assert(bool b); + static std::string stmtSwitch(std::string s); + static int valueSwitch(std::string s); static void main(); }; diff --git a/test/unit_testing/tests/Switch/intended/src/Main.cpp b/test/unit_testing/tests/Switch/intended/src/Main.cpp index 52bacbd7..f19bd743 100644 --- a/test/unit_testing/tests/Switch/intended/src/Main.cpp +++ b/test/unit_testing/tests/Switch/intended/src/Main.cpp @@ -14,6 +14,54 @@ void _Main::Main_Fields_::assert(bool b) { }; } +std::string _Main::Main_Fields_::stmtSwitch(std::string s) { + std::string matched = "?"s; + + auto __temp = s; + switch(__temp.size()) { + case 1: { + if(__temp == "A"s) { + matched = "A"s; + } else if(__temp == "B"s) { + matched = "B"s; + } else { + matched = "default"s; + } + break; + } + default: { + matched = "default"s; + break; + } + }; + + return matched; +} + +int _Main::Main_Fields_::valueSwitch(std::string s) { + int tempResult = 0; + + auto __temp = s; + switch(__temp.size()) { + case 1: { + if(__temp == "A"s) { + tempResult = 1; + } else if(__temp == "B"s) { + tempResult = 2; + } else { + tempResult = 99; + } + break; + } + default: { + tempResult = 99; + break; + } + }; + + return tempResult; +} + void _Main::Main_Fields_::main() { int a = 123; @@ -76,6 +124,11 @@ void _Main::Main_Fields_::main() { default: {} }; + _Main::Main_Fields_::assert(_Main::Main_Fields_::stmtSwitch("C"s) == "default"s); + _Main::Main_Fields_::assert(_Main::Main_Fields_::stmtSwitch("A"s) == "A"s); + _Main::Main_Fields_::assert(_Main::Main_Fields_::valueSwitch("C"s) == 99); + _Main::Main_Fields_::assert(_Main::Main_Fields_::valueSwitch("B"s) == 2); + int tempNumber = 0; switch(a) {