Seems like a smart solution! Could we maybe optimize this a little bit by aggregating empty fallthrough cases into a single if clause? Would be a nice snapshot test too. I'm thinking of something like:

switch (a) {
    case b:
    case c:
    case d:
        foo();
        break;
    default:
        bar();
        break;
}

translating into something like:

if (a == b or a == c or a == d) then
    foo()
else
    bar();
end

Great minds think alike! ;)

I didn't think of the empty case grouping, but I was onto this idea that if I actually didn't use an else statement at all, I could handle all cases with if and or. for example...

switch (a) {
    case b:
    case c:
        foo();
    case d:
        bar();
        break;
    default:
        baz();
        break;
}

if (a == b or a == c) then
    foo()
end
if (a == b or a == c or a == d) then
    bar()
end
if (not (a == b or a == c or a == d))
    baz();
end

With this approach, you still collect the fallthroughs, but then instead of duplicating the code blocks, you let the if's cascade, similar to how it was done with goto statements. I am fairly certain that the hoisting would allow for this to result in the same execution as the code duplication technique.

The default case would then always have to be the not of all the cases present in the switch. This allows for default to appear anywhere in the execution order, by prepending with or's for the fallthrough of default as well... This could be further optimized by storing the right-hand side of the compare in a table as the key where the absence of the key means it is the default case. Special care would need to be taken around nil, as that can't be a key in the table, but could be a case clause.

Test added in 9172e88 with the contents of #1099 (comment) to ensure proper scoping. implementation on master fails this test.

@tomblind repeat...until in 504fd1a

Ready for review.
Any other tests we should run through for good measure?

This is looking good overall. My only remaining concern is that default clauses fallen into at the end produces an extra block that will never get hit. This doesn't technically break anything, it's just weird.

switch (x) {
    case 1:
        console.log(1);
    default:
        console.log("default");
}

local ____switch2 = x
local ____cond2 = ____switch2 == 1
if ____cond2 then
    print(1)
end
if ____cond2 then
    print("default")
end
if not ____cond2 then
    print("default")
end

Sorry, but I came up with another way to break this 😅

Trailing empty cases at the end of the switch are not currently evaluated:

let y = 0;
function foo() {
    return y++;
}

let x = 1;
switch (x) {
    case foo():
        console.log("foo");
    case foo():
}
console.log(y); // should be 2, but is currently 1

@tomblind I believe this is the solution to the extra clause emitting for default. Let me know.

There's still a redundant default clause, although in a different way now:

local ____switch2 = x
local ____cond2 = ____switch2 == 1
if ____cond2 then
    print(1)
end
if ____cond2 then
    print("default")
    break
end
do
    print("default")
end

Would just skipping the condition default clause if it's the last clause in the switch solve this? Or are there other cases that would break?

You know more tests might know the answer... ;) Let us find out.

Initially, I thought I would need to keep the default there as it falls through into the default, but now that we are adding break to the final clause, then we can naturally fall through to the final default clause, like suggested. No problem with side effects either as default will never evaluate a new condition, and so long as we hoast the evaluation to the initial statement, I think we are golden. Tests are passing too.

Sorry, found another way to break it:

let y = 0;
function foo() {
    return y++;
}

let x = 0;
switch (x) {
    case 1:
        console.log(1);
    case foo():
    default:
        console.log("default");
}
console.log(y); // should print 1, but doesn't because foo() is never evaluated

maybe instead of trying to coalesce the conditions for empty statements, we can just eval them separately to keep the logic simpler:

switch (x) {
    case 1:
    case 2:
    case 3:
        // clause for case 3
}

____cond = ____cond or ____switch == 1
____cond = ____cond or ____switch == 2
____cond = ____cond or ____switch == 3
if ____cond then
    --clause for case 3
end

@tomblind can't believe that slipped through the tests. I swear that WAS working in one of these revisions.
Also, no way, too much time was put into making perfect Lua output. We must push forth.

@tomblind I mislead myself with my own comment, and after reading through the code again immediately realized the missing condition. I don't think I have it in me to hoist the second check to the initial condition in this special case though, as it would require another refactor or complicate it further. LMK.