From 27401f0ef8c6c484b4916522bb9435ed96ad76b3 Mon Sep 17 00:00:00 2001 From: moz Date: Fri, 28 Jul 2017 17:46:09 -0700 Subject: [PATCH] Remove unnecessary fallthrough case labels eg. switch('foo') { case 'foo': case 'bar': alert(); } becomes alert(); ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=163541301 --- .../jscomp/PeepholeRemoveDeadCode.java | 30 +++++-- .../jscomp/PeepholeRemoveDeadCodeTest.java | 81 ++++++++++++++----- 2 files changed, 84 insertions(+), 27 deletions(-) diff --git a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java index 25271ffa341..51e7d118d5d 100644 --- a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java +++ b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java @@ -374,7 +374,10 @@ private Node tryOptimizeSwitch(Node n) { // Generally, it is unsafe to remove other cases when the default case is not the last one. if (defaultCase == null || n.getLastChild().isDefaultCase()) { - Node cond = n.getFirstChild(), prev = null, next = null, cur; + Node cond = n.getFirstChild(); + Node prev = null; + Node next = null; + Node cur; for (cur = cond.getNext(); cur != null; cur = next) { next = cur.getNext(); @@ -404,18 +407,33 @@ private Node tryOptimizeSwitch(Node n) { removeCase(n, cur); } } - if (caseMatches != TernaryValue.UNKNOWN) { - Node block, lastStm; + if (cur != null && caseMatches == TernaryValue.TRUE) { // Skip cases until you find one whose last stm is a removable break + Node matchingCase = cur; + Node matchingCaseBlock = matchingCase.getLastChild(); while (cur != null) { - block = cur.getLastChild(); - lastStm = block.getLastChild(); - cur = cur.getNext(); + Node block = cur.getLastChild(); + Node lastStm = block.getLastChild(); + boolean isLastStmRemovableBreak = false; if (lastStm != null && isExit(lastStm)) { removeIfUnnamedBreak(lastStm); + isLastStmRemovableBreak = true; + } + next = cur.getNext(); + // Remove the fallthrough case labels + if (cur != matchingCase) { + while (block.hasChildren()) { + matchingCaseBlock.addChildToBack(block.getFirstChild().detach()); + } + cur.detach(); + reportCodeChange(); + } + cur = next; + if (isLastStmRemovableBreak) { break; } } + // Remove any remaining cases for (; cur != null; cur = next) { next = cur.getNext(); diff --git a/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java b/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java index bf75adfc161..02e19ac8b06 100644 --- a/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java +++ b/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java @@ -407,22 +407,39 @@ public void testOptimizeSwitch() { " break;\n" + "}", ""); - foldSame("switch ('fallThru') {\n" + - "case 'fallThru':\n" + - " if (foo(123) > 0) {\n" + - " foobar(1);\n" + - " break;\n" + - " }\n" + - " foobar(2);\n" + - "case 'bar':\n" + - " bar();\n" + - "}"); - foldSame("switch ('fallThru') {\n" + - "case 'fallThru':\n" + - " foo();\n" + - "case 'bar':\n" + - " bar();\n" + - "}"); + fold( + LINE_JOINER.join( + "switch ('fallThru') {", + "case 'fallThru':", + " if (foo(123) > 0) {", + " foobar(1);", + " break;", + " }", + " foobar(2);", + "case 'bar':", + " bar();", + "}"), + LINE_JOINER.join( + "switch ('fallThru') {", + "case 'fallThru':", + " if (foo(123) > 0) {", + " foobar(1);", + " break;", + " }", + " foobar(2);", + " bar();", + "}")); + fold( + LINE_JOINER.join( + "switch ('fallThru') {", + "case 'fallThru':", + " foo();", + "case 'bar':", + " bar();", + "}"), + LINE_JOINER.join( + "foo();", + "bar();")); fold( LINE_JOINER.join( "switch ('hasDefaultCase') {", @@ -496,11 +513,14 @@ public void testOptimizeSwitch() { "case '\\u000B':\n" + " foo();\n" + "}"); - foldSame("switch ('empty') {\n" + - "case 'empty':\n" + - "case 'foo':\n" + - " foo();\n" + - "}"); + fold( + LINE_JOINER.join( + "switch ('empty') {", + "case 'empty':", + "case 'foo':", + " foo();", + "}"), + "foo()"); fold( LINE_JOINER.join( @@ -547,6 +567,25 @@ public void testOptimizeSwitch2() { "outer: {f(); break outer;}"); } + public void testOptimizeSwitch3() { + fold( + LINE_JOINER.join( + "switch (1) {", + " case 1:", + " case 2:", + " case 3: {", + " break;", + " }", + " case 4:", + " case 5:", + " case 6:", + " default:", + " fail('Should not get here');", + " break;", + "}"), + ""); + } + public void testOptimizeSwitchWithLabellessBreak() { test( LINE_JOINER.join(