Skip to content

Commit

Permalink
Ensure default case folding only happens when default is last (#1084)
Browse files Browse the repository at this point in the history 
* Ensure default case folding only happens when default is last

I failed to consider if multiple branches could evaluate to the test condition. Eg,

```js
switch (true) {
    case definitely_true:
    default:
      return 'definitely';
    case maybe_true:
      return 'maybe';
}
```

Here, both `definitely_true` and `maybe_true` can be `=== true`. And we can't eliminate the `case definitely_true:`, because it prevents us from falling into the `maybe_true` branch (when it is true).

* Add test cases for branch reordering
  • Loading branch information
@jridgewell
jridgewell authored Oct 28, 2021
1 parent e61a299 commit aadf03d
Show file tree
Hide file tree
Showing 2 changed files with 247 additions and 7 deletions.
6 changes: 5 additions & 1 deletion lib/compress/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1694,12 +1694,16 @@ def_optimize(AST_Switch, function(self, compressor) {


// Prune side-effect free branches that fall into default.
if (default_or_exact) {
DEFAULT: if (default_or_exact) {
let default_index = body.indexOf(default_or_exact);
let default_body_index = default_index;
for (; default_body_index < body.length - 1; default_body_index++) {
if (!is_inert_body(body[default_body_index])) break;
}
if (default_body_index < body.length - 1) {
break DEFAULT;
}

let side_effect_index = body.length - 1;
for (; side_effect_index >= 0; side_effect_index--) {
let branch = body[side_effect_index];
Expand Down
248 changes: 242 additions & 6 deletions test/compress/switch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,7 @@ collapse_into_default_12: {
case bar:
bar();
case qux:
case 'qux':
default:
other();
case 'baz':
Expand Down Expand Up @@ -1283,6 +1284,7 @@ collapse_into_default_14: {
bar();
default:
case qux:
case 'qux':
other();
case 'baz':
baz();
Expand Down Expand Up @@ -1354,6 +1356,7 @@ collapse_into_default_16: {
bar();
case qux:
default:
case 'qux':
other();
case 'baz':
baz();
Expand Down Expand Up @@ -2687,6 +2690,8 @@ collapse_same_branches_as_default_not_in_a_row: {
}
expect: {
switch (id(1)) {
case 1:
case 4:
default:
console.log("PASS");
break;
Expand Down Expand Up @@ -2861,9 +2866,16 @@ trim_side_effect_free_branches_falling_into_default: {
}
}
expect: {
if (2 !== id(1))
console.log("PASS default");
console.log("PASS 2")
switch (id(1)) {
case 0:
"no side effect"
case 1:
// Not here either
default:
console.log("PASS default")
case 2:
console.log("PASS 2")
}
}
}

Expand All @@ -2884,9 +2896,15 @@ trim_side_effect_free_branches_falling_into_default_2: {
}
}
expect: {
if (2 !== id(1))
console.log("PASS default");
console.log("PASS 2")
switch (id(1)) {
default:
case 0:
"no side effect"
case 1:
console.log("PASS default")
case 2:
console.log("PASS 2")
}
}
}

Expand Down Expand Up @@ -2969,3 +2987,221 @@ turn_into_if_2: {
}
expect_stdout: "PASS"
}

issue_1083_1: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
default:
console.log("PASS");
break;
case maybe_true:
console.log("FAIL");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
default:
console.log("PASS");
break;
case maybe_true:
console.log("FAIL");
}
}
test(true, false);
test(true, true);
}
expect_stdout: ["PASS", "PASS"]
}

issue_1083_2: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
default:
console.log("PASS");
break;
case maybe_true:
console.log("FAIL");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
default:
console.log("PASS");
break;
case maybe_true:
console.log("FAIL");
}
}
test(true, false);
test(true, true);
}
expect_stdout: ["PASS", "PASS"]
}

issue_1083_3: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
case maybe_true:
console.log("maybe");
break;
default:
case definitely_true:
console.log("definitely");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
if (true === maybe_true) console.log("maybe");
else console.log("definitely");
}
test(true, false);
test(true, true);
}
expect_stdout: ["definitely", "maybe"]
}

issue_1083_4: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
case maybe_true:
console.log("maybe");
break;
case definitely_true:
default:
console.log("definitely");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
if (true === maybe_true) console.log("maybe");
else console.log("definitely");
}
test(true, false);
test(true, true);
}
expect_stdout: ["definitely", "maybe"]
}

issue_1083_5: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
default:
console.log("definitely");
break;
case maybe_true:
console.log("maybe");
break;
case definitely_true:
console.log("definitely");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
switch (true) {
default:
console.log("definitely");
break;
case maybe_true:
console.log("maybe");
break;
case definitely_true:
console.log("definitely");
}
}
test(true, false);
test(true, true);
}
expect_stdout: ["definitely", "maybe"]
}

issue_1083_6: {
options = {
switches: true,
dead_code: true
}
input: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
console.log("definitely");
break;
case maybe_true:
console.log("maybe");
break;
default:
console.log("definitely");
break;
}
}
test(true, false);
test(true, true);
}
expect: {
function test(definitely_true, maybe_true) {
switch (true) {
case definitely_true:
console.log("definitely");
break;
case maybe_true:
console.log("maybe");
break;
default:
console.log("definitely");
}
}
test(true, false);
test(true, true);
}
expect_stdout: ["definitely", "definitely"]
}

0 comments on commit aadf03d

Please sign in to comment.