Skip to content

Commit 8d2e9ac

Browse files
eric-milleseric-milles
committed
GROOVY-11840: SC: implicit-this reference to outer class static field
3_0_X backport
1 parent 34e9abe commit 8d2e9ac

5 files changed

Lines changed: 130 additions & 78 deletions

File tree

‎src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.codehaus.groovy.classgen.AsmClassGenerator;
4545
import org.codehaus.groovy.classgen.Verifier;
4646
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
47-
import org.codehaus.groovy.classgen.asm.CallSiteWriter;
4847
import org.codehaus.groovy.classgen.asm.CompileStack;
4948
import org.codehaus.groovy.classgen.asm.ExpressionAsVariableSlot;
5049
import org.codehaus.groovy.classgen.asm.InvocationWriter;
@@ -527,6 +526,11 @@ private void visitArgument(final Expression argumentExpr, final ClassNode parame
527526
}
528527
}
529528

529+
@Override
530+
protected boolean makeCachedCall(final Expression origin, final ClassExpression sender, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis, final boolean containsSpreadExpression) {
531+
return false;
532+
}
533+
530534
@Override
531535
public void makeCall(final Expression origin, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
532536
if (origin.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION) != null) {
@@ -646,17 +650,11 @@ public void makeCall(final Expression origin, final Expression receiver, final E
646650
mv.visitInsn(ACONST_NULL);
647651
mv.visitLabel(endof);
648652
} else {
649-
if (origin instanceof AttributeExpression && (adapter == AsmClassGenerator.getField || adapter == AsmClassGenerator.getGroovyObjectField)) {
650-
CallSiteWriter callSiteWriter = controller.getCallSiteWriter();
651-
String fieldName = ((AttributeExpression) origin).getPropertyAsString();
652-
if (fieldName != null && callSiteWriter instanceof StaticTypesCallSiteWriter) {
653-
ClassNode receiverType = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
654-
if (((StaticTypesCallSiteWriter) callSiteWriter).makeGetField(receiver, receiverType, fieldName, safe, false)) {
655-
return;
656-
}
657-
}
653+
boolean tryGetField = (adapter == AsmClassGenerator.getField
654+
|| adapter == AsmClassGenerator.getGroovyObjectField);
655+
if (!tryGetField || !makeGetField(origin, receiver, safe)) { // GETFIELD or GETSTATIC
656+
super.makeCall(origin, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
658657
}
659-
super.makeCall(origin, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
660658
}
661659
}
662660

@@ -690,6 +688,20 @@ private boolean tryImplicitReceiver(final Expression origin, final Expression me
690688
return false;
691689
}
692690

691+
private boolean makeGetField(final Expression origin, final Expression receiver, final boolean safe) {
692+
if (origin instanceof AttributeExpression && controller.getCallSiteWriter() instanceof StaticTypesCallSiteWriter) {
693+
String fieldName = ((AttributeExpression) origin).getPropertyAsString();
694+
if (fieldName != null) {
695+
ClassNode receiverType = receiver.getType();
696+
if (!(receiver instanceof ClassExpression)) { // GROOVY-11840
697+
receiverType = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
698+
}
699+
return ((StaticTypesCallSiteWriter) controller.getCallSiteWriter()).makeGetField(receiver, receiverType, fieldName, safe, /*implicitThis:*/false);
700+
}
701+
}
702+
return false;
703+
}
704+
693705
private class CheckcastReceiverExpression extends Expression {
694706
private final Expression receiver;
695707
private final MethodNode target;
@@ -763,9 +775,4 @@ public ClassNode getType() {
763775
return type;
764776
}
765777
}
766-
767-
@Override
768-
protected boolean makeCachedCall(final Expression origin, final ClassExpression sender, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis, final boolean containsSpreadExpression) {
769-
return false;
770-
}
771778
}

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

Lines changed: 35 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import static org.objectweb.asm.Opcodes.DUP;
4545
import static org.objectweb.asm.Opcodes.GOTO;
4646
import static org.objectweb.asm.Opcodes.ICONST_0;
47+
import static org.objectweb.asm.Opcodes.ICONST_1;
4748
import static org.objectweb.asm.Opcodes.IFNONNULL;
4849
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
4950
import static org.objectweb.asm.Opcodes.POP;
@@ -65,12 +66,10 @@ Expression transformBooleanExpression(final BooleanExpression boolX) {
6566
} while (expr instanceof BooleanExpression);
6667

6768
if (!(expr instanceof BinaryExpression)) {
68-
expr = transformer.transform(expr);
69-
ClassNode type = transformer.getTypeChooser().resolveType(expr, transformer.getClassNode());
70-
Expression opt = new OptimizingBooleanExpression(expr, type);
71-
if (reverse) opt = new NotExpression(opt);
72-
opt.setSourcePosition(boolX);
73-
return opt;
69+
expr = new OptimizingBooleanExpression(transformer.transform(expr));
70+
if (reverse) expr = new NotExpression(expr);
71+
expr.setSourcePosition(boolX);
72+
return expr;
7473
}
7574

7675
return transformer.superTransform(boolX);
@@ -80,17 +79,13 @@ Expression transformBooleanExpression(final BooleanExpression boolX) {
8079

8180
private static class OptimizingBooleanExpression extends BooleanExpression {
8281

83-
private final ClassNode type;
84-
85-
OptimizingBooleanExpression(final Expression expression, final ClassNode type) {
82+
OptimizingBooleanExpression(final Expression expression) {
8683
super(expression);
87-
// we must use the redirect node, otherwise InnerClassNode would not have the "correct" type
88-
this.type = type.redirect();
8984
}
9085

9186
@Override
9287
public Expression transformExpression(final ExpressionTransformer transformer) {
93-
Expression opt = new OptimizingBooleanExpression(transformer.transform(getExpression()), type);
88+
Expression opt = new OptimizingBooleanExpression(transformer.transform(getExpression()));
9489
opt.setSourcePosition(this);
9590
opt.copyNodeMetaData(this);
9691
return opt;
@@ -103,48 +98,40 @@ public void visit(final GroovyCodeVisitor visitor) {
10398
MethodVisitor mv = controller.getMethodVisitor();
10499
OperandStack os = controller.getOperandStack();
105100

106-
if (ClassHelper.Boolean_TYPE.equals(type)) {
107-
getExpression().visit(visitor);
108-
Label unbox = new Label();
109-
Label exit = new Label();
110-
// check for null
101+
int mark = os.getStackLength();
102+
getExpression().visit(visitor);
103+
104+
ClassNode type = os.getTopOperand(); // GROOVY-6270, GROOVY-9501, GROOVY-9569, GROOVY-10920, GROOVY-11840
105+
106+
if (ClassHelper.isPrimitiveType(type)) {
107+
BytecodeHelper.convertPrimitiveToBoolean(mv, type);
108+
} else {
111109
mv.visitInsn(DUP);
112-
mv.visitJumpInsn(IFNONNULL, unbox);
110+
Label end = new Label();
111+
Label asBoolean = new Label();
112+
mv.visitJumpInsn(IFNONNULL, asBoolean);
113+
114+
// null => false
113115
mv.visitInsn(POP);
114116
mv.visitInsn(ICONST_0);
115-
mv.visitJumpInsn(GOTO, exit);
116-
mv.visitLabel(unbox);
117-
if (!os.getTopOperand().equals(type)) BytecodeHelper.doCast(mv, type); // GROOVY-6270
118-
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Boolean", "booleanValue", "()Z", false);
119-
mv.visitLabel(exit);
120-
os.replace(ClassHelper.boolean_TYPE);
121-
return;
122-
}
123-
124-
if (ClassHelper.isPrimitiveType(type) && !ClassHelper.VOID_TYPE.equals(type)) { // GROOVY-10920
125-
getExpression().visit(visitor);
126-
if (ClassHelper.boolean_TYPE.equals(type)) {
127-
os.doGroovyCast(ClassHelper.boolean_TYPE);
128-
return;
117+
mv.visitJumpInsn(GOTO, end);
118+
119+
mv.visitLabel(asBoolean);
120+
if (ClassHelper.Boolean_TYPE.equals(type)) {
121+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Boolean", "booleanValue", "()Z", false);
122+
} else if (replaceAsBooleanWithCompareToNull(type, controller.getSourceUnit().getClassLoader())) {
123+
// value => true
124+
mv.visitInsn(POP);
125+
mv.visitInsn(ICONST_1);
129126
} else {
130-
// in case of null safe invocation, it is possible that what was supposed to be a primitive type
131-
// becomes the "null" constant, so we need to recheck
132-
ClassNode top = controller.getOperandStack().getTopOperand();
133-
if (ClassHelper.isPrimitiveType(top)) {
134-
BytecodeHelper.convertPrimitiveToBoolean(mv, top);
135-
os.replace(ClassHelper.boolean_TYPE);
136-
return;
137-
}
127+
os.castToBool(mark, true);
138128
}
129+
mv.visitLabel(end);
139130
}
140-
141-
if (replaceAsBooleanWithCompareToNull(type, controller.getSourceUnit().getClassLoader())) {
142-
new CompareToNullExpression(getExpression(), false).visit(visitor);
143-
return;
144-
}
131+
os.replace(ClassHelper.boolean_TYPE);
132+
} else {
133+
super.visit(visitor);
145134
}
146-
147-
super.visit(visitor);
148135
}
149136

150137
/**
@@ -158,7 +145,7 @@ public void visit(final GroovyCodeVisitor visitor) {
158145
private static boolean replaceAsBooleanWithCompareToNull(final ClassNode type, final ClassLoader dgmProvider) {
159146
if (type.getMethod("asBoolean", Parameter.EMPTY_ARRAY) != null) {
160147
// GROOVY-10711
161-
} else if (Modifier.isFinal(type.getModifiers()) || isEffectivelyFinal(type)) {
148+
} else if (Modifier.isFinal(type.getModifiers()) || isEffectivelyFinal(type.redirect())) {
162149
List<MethodNode> asBoolean = findDGMMethodsByNameAndArguments(dgmProvider, type, "asBoolean", ClassNode.EMPTY_ARRAY);
163150
if (asBoolean.size() == 1) {
164151
MethodNode theAsBoolean = asBoolean.get(0);

‎src/test/gls/innerClass/InnerClassTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,8 @@ final class InnerClassTest {
597597
@Override
598598
void run() {
599599
try {
600-
if (!flag) {
601-
// do work
600+
if (flag) {
601+
assert false : 'boolean conversion'
602602
}
603603
} catch (e) {
604604
error = e

‎src/test/groovy/transform/stc/ClosuresSTCTest.groovy‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,26 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
660660
'''
661661
}
662662

663+
// GROOVY-11840
664+
void testAccessStaticFieldFromNestedClosure2() {
665+
assertScript '''
666+
class C {
667+
private static final boolean FLAG = false
668+
669+
static main(args) {
670+
(1..100).each {
671+
if (FLAG) {
672+
assert false : 'boolean conversion'
673+
}
674+
if (FLAG == true) {
675+
assert false : 'boolean comparison'
676+
}
677+
}
678+
}
679+
}
680+
'''
681+
}
682+
663683
// GROOVY-11360
664684
void testLexicalScopeVersusGetDynamicProperty() {
665685
assertScript '''

‎src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,36 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes
195195
''')
196196
assert bytecode.hasSequence([
197197
'ALOAD 1',
198+
'DUP',
199+
'IFNONNULL L1',
200+
'POP',
201+
'ICONST_0',
202+
'GOTO L2',
203+
'L1',
198204
config.indyEnabled ? 'INVOKEDYNAMIC cast(Ljava/lang/Object;)Z' : 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z'
199205
])
200206
}
201207

208+
void testNoGroovyTruthOptimizationForString() {
209+
def bytecode = compile(method:'m', '''
210+
@groovy.transform.CompileStatic
211+
void m(String x) {
212+
if (x) {
213+
}
214+
}
215+
''')
216+
assert bytecode.hasSequence([
217+
'ALOAD 1',
218+
'DUP',
219+
'IFNONNULL L1',
220+
'POP',
221+
'ICONST_0',
222+
'GOTO L2',
223+
'L1',
224+
config.indyEnabled ? 'INVOKEDYNAMIC cast(Ljava/lang/String;)Z' : 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z'
225+
])
226+
}
227+
202228
void testGroovyTruthOptimizationForFinalClass() {
203229
def bytecode = compile(method:'m', '''
204230
final class A {
@@ -211,13 +237,13 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes
211237
''')
212238
assert bytecode.hasSequence([
213239
'ALOAD 1',
214-
'IFNULL L1',
215-
'ICONST_1',
216-
'GOTO L2',
217-
'L1',
240+
'DUP',
241+
'IFNONNULL L1',
242+
'POP',
218243
'ICONST_0',
219-
'L2',
220-
'IFEQ L3'
244+
'GOTO L2',
245+
'POP',
246+
'ICONST_1'
221247
])
222248
if (config.indyEnabled)
223249
assert !bytecode.hasSequence(['INVOKEDYNAMIC cast(LA$B;)Z'])
@@ -237,13 +263,13 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes
237263
''')
238264
assert bytecode.hasSequence([
239265
'ALOAD 1',
240-
'IFNULL L1',
241-
'ICONST_1',
242-
'GOTO L2',
243-
'L1',
266+
'DUP',
267+
'IFNONNULL L1',
268+
'POP',
244269
'ICONST_0',
245-
'L2',
246-
'IFEQ L3'
270+
'GOTO L2',
271+
'POP',
272+
'ICONST_1'
247273
])
248274
if (config.indyEnabled)
249275
assert !bytecode.hasSequence(['INVOKEDYNAMIC cast(LA$B;)Z'])
@@ -263,6 +289,12 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes
263289
''')
264290
assert bytecode.hasSequence([
265291
'ALOAD 1',
292+
'DUP',
293+
'IFNONNULL L1',
294+
'POP',
295+
'ICONST_0',
296+
'GOTO L2',
297+
'L1',
266298
config.indyEnabled ? 'INVOKEDYNAMIC cast(LA$B;)Z' : 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z'
267299
])
268300
}
@@ -285,6 +317,12 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes
285317
''')
286318
assert bytecode.hasSequence([
287319
'ALOAD 1',
320+
'DUP',
321+
'IFNONNULL L1',
322+
'POP',
323+
'ICONST_0',
324+
'GOTO L2',
325+
'L1',
288326
config.indyEnabled ? 'INVOKEDYNAMIC cast(LC;)Z' : 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z'
289327
])
290328
}

0 commit comments

Comments
 (0)