Skip to content

Commit 7ebaf4a

Browse files
eric-milleseric-milles
committed
GROOVY-4727, GROOVY-9373, GROOVY-9880, GROOVY-9896
GROOVY-9126 rollback for Groovy 2.5/3.0
1 parent a517299 commit 7ebaf4a

7 files changed

Lines changed: 189 additions & 70 deletions

File tree

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java‎

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2496,11 +2496,113 @@ public void testAbstractClass_GRE274_2() {
24962496
"----------\n");
24972497
}
24982498

2499+
@Test
2500+
public void testSwitchCases1() {
2501+
//@formatter:off
2502+
String[] sources = {
2503+
"X.groovy",
2504+
"def foo(p) {\n" +
2505+
" switch (p) {\n" +
2506+
" case 1:\n" +
2507+
" 'a'\n" +
2508+
" break\n" +
2509+
" case 2:\n" +
2510+
" if (false) 'b'\n" +
2511+
" else 'c'\n" +
2512+
" break\n" +
2513+
" case 3:\n" +
2514+
" 'skip'\n" +
2515+
" default:\n" +
2516+
" 'd'\n" +
2517+
" }\n" +
2518+
"}\n" +
2519+
"print foo(1)\n" +
2520+
"print foo(2)\n" +
2521+
"print foo(3)\n" +
2522+
"print foo(4)\n",
2523+
};
2524+
//@formatter:on
2525+
2526+
runConformTest(sources, "acdd");
2527+
}
2528+
2529+
@Test // GROOVY-9896
2530+
public void testSwitchCases2() {
2531+
//@formatter:off
2532+
String[] sources = {
2533+
"X.groovy",
2534+
"def foo(p) {\n" +
2535+
" switch (p) {\n" +
2536+
" case 1:\n" +
2537+
" 'a'\n" +
2538+
" break\n" +
2539+
" case 2:\n" +
2540+
" 'b'\n" +
2541+
" break\n" +
2542+
" case 3:\n" +
2543+
" 'c'\n" +
2544+
" }\n" +
2545+
"}\n" +
2546+
"print foo(1)\n" +
2547+
"print foo(2)\n" +
2548+
"print foo(3)\n" +
2549+
"print foo(4)\n",
2550+
};
2551+
//@formatter:on
2552+
2553+
runConformTest(sources, "abcnull");
2554+
}
2555+
2556+
@Test // GROOVY-4727
2557+
public void testSwitchCases3() {
2558+
//@formatter:off
2559+
String[] sources = {
2560+
"X.groovy",
2561+
"def foo(x,y) {\n" +
2562+
" switch (x) {\n" +
2563+
" case 'x1':\n" +
2564+
" switch (y) {\n" +
2565+
" case 'y1':\n" +
2566+
" 'r1'\n" +
2567+
" break\n" +
2568+
" case 'y2':\n" +
2569+
" 'r2'\n" +
2570+
" break\n" +
2571+
" }\n" +
2572+
// no break
2573+
" }\n" +
2574+
"}\n" +
2575+
"print foo('x1','y1')\n",
2576+
};
2577+
//@formatter:on
2578+
2579+
runConformTest(sources, "r1");
2580+
}
2581+
2582+
@Test // GROOVY-9880
2583+
public void testBreakAfterIf() {
2584+
//@formatter:off
2585+
String[] sources = {
2586+
"X.groovy",
2587+
"switch ('value') {\n" +
2588+
" case 'value':\n" +
2589+
" print 'foo'\n" +
2590+
" if (false) print 'X'\n" +
2591+
" break\n" +
2592+
" default:\n" +
2593+
" print 'bar'\n" +
2594+
"}\n",
2595+
};
2596+
//@formatter:on
2597+
2598+
runConformTest(sources, "foo"); // not "foobar"
2599+
}
2600+
24992601
@Test
25002602
public void testBreak_GRE290() {
25012603
//@formatter:off
25022604
String[] sources = {
2503-
"p/X.groovy",
2605+
"X.groovy",
25042606
"words: [].each { final item ->\n" +
25052607
" break words\n" +
25062608
"}\n",
@@ -2509,7 +2611,7 @@ public void testBreak_GRE290() {
25092611

25102612
runNegativeTest(sources,
25112613
"----------\n" +
2512-
"1. ERROR in p\\X.groovy (at line 2)\n" +
2614+
"1. ERROR in X.groovy (at line 2)\n" +
25132615
"\tbreak words\n" +
25142616
"\t^^^^^^^^^^^\n" +
25152617
"Groovy:" + (!isParrotParser()

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/ReturnAdder.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.codehaus.groovy.ast.stmt.TryCatchStatement;
3939

4040
import java.util.ArrayList;
41+
import java.util.Iterator;
4142
import java.util.List;
4243

4344
/**
@@ -120,13 +121,9 @@ private Statement addReturnsIfNeeded(Statement statement, VariableScope scope) {
120121
}
121122

122123
if (statement instanceof EmptyStatement) {
123-
/* GRECLIPSE edit -- GROOVY-9373
124124
final ReturnStatement returnStatement = new ReturnStatement(ConstantExpression.NULL);
125125
listener.returnStatementAdded(returnStatement);
126126
return returnStatement;
127-
*/
128-
return statement;
129-
// GRECLIPSE end
130127
}
131128

132129
if (statement instanceof ExpressionStatement) {
@@ -159,11 +156,23 @@ private Statement addReturnsIfNeeded(Statement statement, VariableScope scope) {
159156

160157
if (statement instanceof SwitchStatement) {
161158
SwitchStatement swi = (SwitchStatement) statement;
159+
/* GRECLIPSE edit -- GROOVY-4727, GROOVY-9896
162160
for (CaseStatement caseStatement : swi.getCaseStatements()) {
163161
final Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false);
164162
if (doAdd) caseStatement.setCode(code);
165163
}
166164
final Statement defaultStatement = adjustSwitchCaseCode(swi.getDefaultStatement(), scope, true);
165+
*/
166+
Statement defaultStatement = swi.getDefaultStatement();
167+
List<CaseStatement> caseStatements = swi.getCaseStatements();
168+
for (Iterator<CaseStatement> it = caseStatements.iterator(); it.hasNext();) {
169+
CaseStatement caseStatement = it.next();
170+
Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope,
171+
defaultStatement == EmptyStatement.INSTANCE && !it.hasNext());
172+
if (doAdd) caseStatement.setCode(code);
173+
}
174+
defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true);
175+
// GRECLIPSE end
167176
if (doAdd) swi.setDefaultStatement(defaultStatement);
168177
return swi;
169178
}

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/StatementWriter.java‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,31 +89,16 @@ public void writeBlockStatement(BlockStatement block) {
8989
statement.visit(controller.getAcg());
9090
}
9191
compileStack.pop();
92-
/* GRECLIPSE edit
93-
// GROOVY-7647, GROOVY-9126
94-
if (block.getLastLineNumber() > 0 && !isMethodOrConstructorNonEmptyBlock(block)) {
92+
93+
// GROOVY-7647
94+
if (block.getLastLineNumber() > 0) {
9595
MethodVisitor mv = controller.getMethodVisitor();
9696
Label blockEnd = new Label(); mv.visitLabel(blockEnd);
9797
mv.visitLineNumber(block.getLastLineNumber(), blockEnd);
9898
}
99-
*/
100-
controller.getOperandStack().popDownTo(mark);
101-
}
102-
103-
/* GRECLIPSE edit
104-
private boolean isMethodOrConstructorNonEmptyBlock(BlockStatement block) {
105-
MethodNode methodNode = controller.getMethodNode();
106-
if (null == methodNode) {
107-
methodNode = controller.getConstructorNode();
108-
}
10999

110-
if (null == methodNode || block != methodNode.getCode()) { // check if the block is method/constructor's code
111-
return false;
112-
}
113-
114-
return !block.getStatements().isEmpty();
100+
controller.getOperandStack().popDownTo(mark);
115101
}
116-
*/
117102

118103
public void writeForStatement(ForStatement loop) {
119104
Parameter loopVar = loop.getVariable();

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/AsmClassGenerator.java‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -474,16 +474,13 @@ private void visitStdMethod(final MethodNode node, final boolean isConstructor,
474474
*/
475475
if (code != null) {
476476
code.visit(this);
477-
}
478-
if (!checkIfLastStatementIsReturnOrThrow(code)) {
479-
if (code != null) {
480-
// GROOVY-7647, GROOVY-9373
481-
int line = code.getLastLineNumber();
482-
if (line > controller.getLineNumber()) {
483-
Label label = new Label(); mv.visitLabel(label);
484-
mv.visitLineNumber(line, label); controller.setLineNumber(line);
485-
}
477+
// GROOVY-7647, GROOVY-9373
478+
int line = code.getLastLineNumber();
479+
if (line > controller.getLineNumber()) {
480+
Label label = new Label(); mv.visitLabel(label);
481+
mv.visitLineNumber(line, label); controller.setLineNumber(line);
486482
}
483+
}
487484
// GRECLIPSE end
488485
if (node.isVoidMethod()) {
489486
mv.visitInsn(RETURN);
@@ -501,27 +498,10 @@ private void visitStdMethod(final MethodNode node, final boolean isConstructor,
501498
}
502499
}
503500
// GRECLIPSE add
504-
}
505501
controller.getCompileStack().clear();
506502
// GRECLIPSE end
507503
}
508504

509-
private boolean checkIfLastStatementIsReturnOrThrow(Statement code) {
510-
if (code instanceof BlockStatement) {
511-
BlockStatement blockStatement = (BlockStatement) code;
512-
List<Statement> statementList = blockStatement.getStatements();
513-
int statementCnt = statementList.size();
514-
if (statementCnt > 0) {
515-
Statement lastStatement = statementList.get(statementCnt - 1);
516-
if (lastStatement instanceof ReturnStatement || lastStatement instanceof ThrowStatement) {
517-
return true;
518-
}
519-
}
520-
}
521-
522-
return false;
523-
}
524-
525505
private void visitAnnotationDefaultExpression(final AnnotationVisitor av, final ClassNode type, final Expression exp) {
526506
if (exp instanceof ClosureExpression) {
527507
ClassNode closureClass = controller.getClosureWriter().getOrAddClosureClass((ClosureExpression) exp, ACC_PUBLIC);

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/ReturnAdder.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.codehaus.groovy.ast.stmt.TryCatchStatement;
3737

3838
import java.util.ArrayList;
39+
import java.util.Iterator;
3940
import java.util.List;
4041
import java.util.Objects;
4142

@@ -100,9 +101,6 @@ public void visitMethod(final MethodNode node) {
100101

101102
private Statement addReturnsIfNeeded(final Statement statement, final VariableScope scope) {
102103
if (statement instanceof ReturnStatement || statement instanceof ThrowStatement
103-
// GRECLIPSE add -- GROOVY-9373
104-
|| statement instanceof EmptyStatement
105-
// GRECLIPSE end
106104
|| statement instanceof BytecodeSequence) {
107105
return statement;
108106
}
@@ -142,11 +140,23 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
142140

143141
if (statement instanceof SwitchStatement) {
144142
SwitchStatement switchStatement = (SwitchStatement) statement;
143+
/* GRECLIPSE edit -- GROOVY-4727, GROOVY-9896
145144
for (CaseStatement caseStatement : switchStatement.getCaseStatements()) {
146145
Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false);
147146
if (doAdd) caseStatement.setCode(code);
148147
}
149148
Statement defaultStatement = adjustSwitchCaseCode(switchStatement.getDefaultStatement(), scope, true);
149+
*/
150+
Statement defaultStatement = switchStatement.getDefaultStatement();
151+
List<CaseStatement> caseStatements = switchStatement.getCaseStatements();
152+
for (Iterator<CaseStatement> it = caseStatements.iterator(); it.hasNext();) {
153+
CaseStatement caseStatement = it.next();
154+
Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope,
155+
defaultStatement == EmptyStatement.INSTANCE && !it.hasNext());
156+
if (doAdd) caseStatement.setCode(code);
157+
}
158+
defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true);
159+
// GRECLIPSE end
150160
if (doAdd) switchStatement.setDefaultStatement(defaultStatement);
151161
return switchStatement;
152162
}

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/StatementWriter.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,16 @@ public void writeBlockStatement(final BlockStatement block) {
9393
statement.visit(controller.getAcg());
9494
}
9595
compileStack.pop();
96-
/* GRECLIPSE edit
97-
// GROOVY-7647, GROOVY-9126
98-
if (block.getLastLineNumber() > 0 && !isMethodOrConstructorNonEmptyBlock(block)) {
96+
97+
// GROOVY-7647
98+
if (block.getLastLineNumber() > 0) {
9999
MethodVisitor mv = controller.getMethodVisitor();
100-
Label blockEnd = new Label();
101-
mv.visitLabel(blockEnd);
100+
Label blockEnd = new Label(); mv.visitLabel(blockEnd);
102101
mv.visitLineNumber(block.getLastLineNumber(), blockEnd);
103102
}
104-
*/
105-
controller.getOperandStack().popDownTo(mark);
106-
}
107103

108-
/* GRECLIPSE edit
109-
private boolean isMethodOrConstructorNonEmptyBlock(final BlockStatement block) {
110-
MethodNode methodNode = controller.getMethodNode();
111-
if (methodNode == null) {
112-
methodNode = controller.getConstructorNode();
113-
}
114-
return (methodNode != null && methodNode.getCode() == block && !block.isEmpty());
104+
controller.getOperandStack().popDownTo(mark);
115105
}
116-
*/
117106

118107
public void writeForStatement(final ForStatement statement) {
119108
if (statement.getVariable() == ForStatement.FOR_LOOP_DUMMY) {

0 commit comments

Comments
 (0)