Skip to content

Commit f21db48

Browse files
eric-milleseric-milles
committed
GROOVY-10424
1 parent a086474 commit f21db48

File tree

7 files changed

+602
-3
lines changed

7 files changed

+602
-3
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java

+24
Original file line numberDiff line numberDiff line change
@@ -7074,4 +7074,28 @@ public void testCompileStatic10395b() {
70747074
int pos = result.indexOf("ScriptBytecodeAdapter.compareTo");
70757075
assertTrue(pos < 0);
70767076
}
7077+
7078+
@Test
7079+
public void testCompileStatic10424() {
7080+
//@formatter:off
7081+
String[] sources = {
7082+
"Main.groovy",
7083+
"@groovy.transform.CompileStatic\n" +
7084+
"class Outer {\n" +
7085+
" private static class Inner {\n" +
7086+
" static class Three {}\n" +
7087+
" }\n" +
7088+
" void test() {\n" +
7089+
" def inner = new Inner()\n" +
7090+
" if (inner) {\n" + // optimized boolean expression; StackOverflowError
7091+
" print 'works'\n" +
7092+
" }\n" +
7093+
" }\n" +
7094+
"}\n" +
7095+
"new Outer().test()\n",
7096+
};
7097+
//@formatter:on
7098+
7099+
runConformTest(sources, "works");
7100+
}
70777101
}

‎base/org.codehaus.groovy25/.checkstyle

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@
8080
<file-match-pattern match-pattern="groovy/transform/TupleConstructorASTTransformation.java" include-pattern="false" />
8181
<file-match-pattern match-pattern="groovy/transform/sc/StaticCompilationVisitor.java" include-pattern="false" />
8282
<file-match-pattern match-pattern="groovy/transform/sc/TemporaryVariableExpression.java" include-pattern="false" />
83-
<file-match-pattern match-pattern="groovy/transform/sc/transformers/(Binary|MethodCall)ExpressionTransformer.java" include-pattern="false" />
83+
<file-match-pattern match-pattern="groovy/transform/sc/transformers/(Binary|Boolean|MethodCall)ExpressionTransformer.java" include-pattern="false" />
8484
<file-match-pattern match-pattern="groovy/transform/sc/transformers/ConstructorCallTransformer.java" include-pattern="false" />
8585
<file-match-pattern match-pattern="groovy/transform/sc/transformers/CompareToNullExpression.java" include-pattern="false" />
8686
<file-match-pattern match-pattern="groovy/transform/sc/transformers/StaticCompilationTransformer.java" include-pattern="false" />

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java

+211
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.codehaus.groovy.transform.sc.transformers;
20+
21+
import org.codehaus.groovy.ast.ClassHelper;
22+
import org.codehaus.groovy.ast.ClassNode;
23+
import org.codehaus.groovy.ast.GroovyCodeVisitor;
24+
import org.codehaus.groovy.ast.InnerClassNode;
25+
import org.codehaus.groovy.ast.MethodNode;
26+
import org.codehaus.groovy.ast.expr.BinaryExpression;
27+
import org.codehaus.groovy.ast.expr.BooleanExpression;
28+
import org.codehaus.groovy.ast.expr.Expression;
29+
import org.codehaus.groovy.ast.expr.ExpressionTransformer;
30+
import org.codehaus.groovy.ast.expr.NotExpression;
31+
import org.codehaus.groovy.classgen.AsmClassGenerator;
32+
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
33+
import org.codehaus.groovy.classgen.asm.OperandStack;
34+
import org.codehaus.groovy.classgen.asm.WriterController;
35+
import org.codehaus.groovy.classgen.asm.sc.StaticTypesTypeChooser;
36+
import org.codehaus.groovy.transform.stc.ExtensionMethodNode;
37+
import groovyjarjarasm.asm.Label;
38+
import groovyjarjarasm.asm.MethodVisitor;
39+
40+
import java.lang.reflect.Modifier;
41+
import java.util.Iterator;
42+
import java.util.List;
43+
44+
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsByNameAndArguments;
45+
import static groovyjarjarasm.asm.Opcodes.DCMPG;
46+
import static groovyjarjarasm.asm.Opcodes.DCONST_0;
47+
import static groovyjarjarasm.asm.Opcodes.DUP;
48+
import static groovyjarjarasm.asm.Opcodes.F2D;
49+
import static groovyjarjarasm.asm.Opcodes.GOTO;
50+
import static groovyjarjarasm.asm.Opcodes.ICONST_0;
51+
import static groovyjarjarasm.asm.Opcodes.IFNONNULL;
52+
import static groovyjarjarasm.asm.Opcodes.INVOKEVIRTUAL;
53+
import static groovyjarjarasm.asm.Opcodes.LCMP;
54+
import static groovyjarjarasm.asm.Opcodes.LCONST_0;
55+
import static groovyjarjarasm.asm.Opcodes.POP;
56+
57+
public class BooleanExpressionTransformer {
58+
private final StaticCompilationTransformer transformer;
59+
60+
public BooleanExpressionTransformer(StaticCompilationTransformer staticCompilationTransformer) {
61+
transformer = staticCompilationTransformer;
62+
}
63+
64+
Expression transformBooleanExpression(final BooleanExpression booleanExpression) {
65+
if (booleanExpression instanceof NotExpression) {
66+
return transformer.superTransform(booleanExpression);
67+
}
68+
final Expression expression = booleanExpression.getExpression();
69+
if (!(expression instanceof BinaryExpression)) {
70+
StaticTypesTypeChooser typeChooser = transformer.getTypeChooser();
71+
final ClassNode type = typeChooser.resolveType(expression, transformer.getClassNode());
72+
BooleanExpression transformed = new OptimizingBooleanExpression(transformer.transform(expression),type);
73+
transformed.setSourcePosition(booleanExpression);
74+
transformed.copyNodeMetaData(booleanExpression);
75+
return transformed;
76+
}
77+
return transformer.superTransform(booleanExpression);
78+
}
79+
80+
private static boolean isExtended(ClassNode owner, Iterator<InnerClassNode> classes) {
81+
while (classes.hasNext()) {
82+
InnerClassNode next = classes.next();
83+
if (next!=owner && next.isDerivedFrom(owner)) return true;
84+
// GRECLIPSE add
85+
if (isExtended(owner,next.getInnerClasses())) return true;
86+
// GRECLIPSE end
87+
}
88+
/* GRECLIPSE edit -- GROOVY-10424
89+
if (owner.getInnerClasses().hasNext()) {
90+
return isExtended(owner, owner.getInnerClasses());
91+
}
92+
*/
93+
return false;
94+
}
95+
96+
private static class OptimizingBooleanExpression extends BooleanExpression {
97+
98+
private final Expression expression;
99+
private final ClassNode type;
100+
101+
public OptimizingBooleanExpression(final Expression expression, final ClassNode type) {
102+
super(expression);
103+
this.expression = expression;
104+
// we must use the redirect node, otherwise InnerClassNode would not have the "correct" type
105+
this.type = type.redirect();
106+
}
107+
108+
@Override
109+
public Expression transformExpression(final ExpressionTransformer transformer) {
110+
Expression ret = new OptimizingBooleanExpression(transformer.transform(expression), type);
111+
ret.setSourcePosition(this);
112+
ret.copyNodeMetaData(this);
113+
return ret;
114+
}
115+
116+
@Override
117+
public void visit(final GroovyCodeVisitor visitor) {
118+
if (visitor instanceof AsmClassGenerator) {
119+
AsmClassGenerator acg = (AsmClassGenerator) visitor;
120+
WriterController controller = acg.getController();
121+
OperandStack os = controller.getOperandStack();
122+
123+
if (type.equals(ClassHelper.boolean_TYPE)) {
124+
expression.visit(visitor);
125+
os.doGroovyCast(ClassHelper.boolean_TYPE);
126+
return;
127+
}
128+
if (type.equals(ClassHelper.Boolean_TYPE)) {
129+
MethodVisitor mv = controller.getMethodVisitor();
130+
expression.visit(visitor);
131+
Label unbox = new Label();
132+
Label exit = new Label();
133+
// check for null
134+
mv.visitInsn(DUP);
135+
mv.visitJumpInsn(IFNONNULL, unbox);
136+
mv.visitInsn(POP);
137+
mv.visitInsn(ICONST_0);
138+
mv.visitJumpInsn(GOTO, exit);
139+
mv.visitLabel(unbox);
140+
// unbox
141+
// GROOVY-6270
142+
if (!os.getTopOperand().equals(type)) BytecodeHelper.doCast(mv, type);
143+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Boolean", "booleanValue", "()Z", false);
144+
mv.visitLabel(exit);
145+
os.replace(ClassHelper.boolean_TYPE);
146+
return;
147+
}
148+
ClassNode top = type;
149+
if (ClassHelper.isPrimitiveType(top)) {
150+
expression.visit(visitor);
151+
// in case of null safe invocation, it is possible that what was supposed to be a primitive type
152+
// becomes the "null" constant, so we need to recheck
153+
top = controller.getOperandStack().getTopOperand();
154+
if (ClassHelper.isPrimitiveType(top)) {
155+
if (top.equals(ClassHelper.int_TYPE) || top.equals(ClassHelper.byte_TYPE)
156+
|| top.equals(ClassHelper.short_TYPE) || top.equals(ClassHelper.char_TYPE)) {
157+
// int on stack
158+
} else if (top.equals(ClassHelper.long_TYPE)) {
159+
MethodVisitor mv = controller.getMethodVisitor();
160+
mv.visitInsn(LCONST_0);
161+
mv.visitInsn(LCMP);
162+
controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
163+
} else if (top.equals(ClassHelper.float_TYPE)) {
164+
MethodVisitor mv = controller.getMethodVisitor();
165+
mv.visitInsn(F2D);
166+
mv.visitInsn(DCONST_0);
167+
mv.visitInsn(DCMPG);
168+
controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
169+
} else if (top.equals(ClassHelper.double_TYPE)) {
170+
MethodVisitor mv = controller.getMethodVisitor();
171+
mv.visitInsn(DCONST_0);
172+
mv.visitInsn(DCMPG);
173+
controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
174+
}
175+
return;
176+
}
177+
}
178+
List<MethodNode> asBoolean = findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), top, "asBoolean", ClassNode.EMPTY_ARRAY);
179+
if (asBoolean.size() == 1) {
180+
MethodNode node = asBoolean.get(0);
181+
if (node instanceof ExtensionMethodNode) {
182+
MethodNode dgmNode = ((ExtensionMethodNode) node).getExtensionMethodNode();
183+
ClassNode owner = dgmNode.getParameters()[0].getType();
184+
if (ClassHelper.OBJECT_TYPE.equals(owner)) {
185+
// we may inline a var!=null check instead of calling a helper method iff
186+
// (1) the class doesn't define an asBoolean method (already tested)
187+
// (2) no subclass defines an asBoolean method
188+
// For (2), we check that we are in one of those cases
189+
// (a) a final class
190+
// (b) a private inner class without subclass
191+
if (Modifier.isFinal(top.getModifiers())
192+
|| (top instanceof InnerClassNode
193+
&& Modifier.isPrivate(top.getModifiers())
194+
&& !isExtended(top, top.getOuterClass().getInnerClasses()))
195+
) {
196+
CompareToNullExpression expr = new CompareToNullExpression(
197+
expression, false
198+
);
199+
expr.visit(acg);
200+
return;
201+
}
202+
}
203+
}
204+
}
205+
super.visit(visitor);
206+
} else {
207+
super.visit(visitor);
208+
}
209+
}
210+
}
211+
}

‎base/org.codehaus.groovy30/.checkstyle

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
<file-match-pattern match-pattern="groovy/transform/NullCheckASTTransformation.java" include-pattern="false" />
6969
<file-match-pattern match-pattern="groovy/transform/TupleConstructorASTTransformation.java" include-pattern="false" />
7070
<file-match-pattern match-pattern="groovy/transform/sc/TemporaryVariableExpression.java" include-pattern="false" />
71-
<file-match-pattern match-pattern="groovy/transform/sc/transformers/(Binary|MethodCall)ExpressionTransformer.java" include-pattern="false" />
71+
<file-match-pattern match-pattern="groovy/transform/sc/transformers/(Binary|Boolean|MethodCall)ExpressionTransformer.java" include-pattern="false" />
7272
<file-match-pattern match-pattern="groovy/transform/sc/transformers/CompareIdentityExpression.java" include-pattern="false" />
7373
<file-match-pattern match-pattern="groovy/transform/sc/transformers/StaticCompilationTransformer.java" include-pattern="false" />
7474
<file-match-pattern match-pattern="groovy/transform/stc/AbstractExtensionMethodCache.java" include-pattern="false" />

0 commit comments

Comments
 (0)