Skip to content

Commit b7ef047

Browse files
aclementaclement
committed
Reduce SpEL compilation restrictions on mathematical expressions
Prior to this change the SpEL compiler would not compile mathematical expressions where the operands were of differing types (e.g. int and double). This required the expression writer to do the conversion in the expression text. For example: T(Integer).valueOf(someInt).doubleValue()/35d With this commit the restriction is lifted and it is more like Java so that you can simply do someInt/35d. The mathematical operators affected are divide/plus/minus/multiply and modulus. This change involved removing some guards so that the exitTypeDescriptor (the trigger for whether compilation is allowed) is set more frequently and enhancing bytecode generation to perform more sophisticated conversion/coercion automatically. Issue: SPR-12789
1 parent 7fbc951 commit b7ef047

File tree

8 files changed

+1263
-110
lines changed

8 files changed

+1263
-110
lines changed

‎spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,125 @@ public static void insertUnboxInsns(MethodVisitor mv, char ch, String stackDescr
209209
}
210210
}
211211

212+
/**
213+
* For numbers, use the appropriate method on the number to convert it to the primitive type requested.
214+
* @param mv the method visitor into which instructions should be inserted
215+
* @param targetDescriptor the primitive type desired as output
216+
* @param stackDescriptor the descriptor of the type on top of the stack
217+
*/
218+
public static void insertUnboxNumberInsns(MethodVisitor mv, char targetDescriptor, String stackDescriptor) {
219+
switch (targetDescriptor) {
220+
case 'D':
221+
if (stackDescriptor.equals("Ljava/lang/Object")) {
222+
mv.visitTypeInsn(CHECKCAST, "java/lang/Number");
223+
}
224+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Number", "doubleValue", "()D", false);
225+
break;
226+
case 'F':
227+
if (stackDescriptor.equals("Ljava/lang/Object")) {
228+
mv.visitTypeInsn(CHECKCAST, "java/lang/Number");
229+
}
230+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Number", "floatValue", "()F", false);
231+
break;
232+
case 'J':
233+
if (stackDescriptor.equals("Ljava/lang/Object")) {
234+
mv.visitTypeInsn(CHECKCAST, "java/lang/Number");
235+
}
236+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Number", "longValue", "()J", false);
237+
break;
238+
case 'I':
239+
if (stackDescriptor.equals("Ljava/lang/Object")) {
240+
mv.visitTypeInsn(CHECKCAST, "java/lang/Number");
241+
}
242+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Number", "intValue", "()I", false);
243+
break;
244+
// does not handle Z, B, C, S
245+
default:
246+
throw new IllegalArgumentException("Unboxing should not be attempted for descriptor '" + targetDescriptor + "'");
247+
}
248+
}
249+
250+
/**
251+
* Insert any necessary numeric conversion bytecodes based upon what is on the stack and the desired target type.
252+
* @param mv the method visitor into which instructions should be placed
253+
* @param targetDescriptor the (primitive) descriptor of the target type
254+
* @param stackDescriptor the descriptor of the operand on top of the stack
255+
*/
256+
public static void insertAnyNecessaryTypeConversionBytecodes(MethodVisitor mv, char targetDescriptor, String stackDescriptor) {
257+
if (CodeFlow.isPrimitive(stackDescriptor)) {
258+
char stackTop = stackDescriptor.charAt(0);
259+
if (stackTop=='I' || stackTop=='B' || stackTop=='S' || stackTop=='C') {
260+
if (targetDescriptor=='D') {
261+
mv.visitInsn(I2D);
262+
}
263+
else if (targetDescriptor=='F') {
264+
mv.visitInsn(I2F);
265+
}
266+
else if (targetDescriptor=='J') {
267+
mv.visitInsn(I2L);
268+
}
269+
else if (targetDescriptor=='I') {
270+
// nop
271+
}
272+
else {
273+
throw new IllegalStateException("cannot get from "+stackTop+" to "+targetDescriptor);
274+
}
275+
}
276+
else if (stackTop=='J') {
277+
if (targetDescriptor=='D') {
278+
mv.visitInsn(L2D);
279+
}
280+
else if (targetDescriptor=='F') {
281+
mv.visitInsn(L2F);
282+
}
283+
else if (targetDescriptor=='J') {
284+
// nop
285+
}
286+
else if (targetDescriptor=='I') {
287+
mv.visitInsn(L2I);
288+
}
289+
else {
290+
throw new IllegalStateException("cannot get from "+stackTop+" to "+targetDescriptor);
291+
}
292+
}
293+
else if (stackTop=='F') {
294+
if (targetDescriptor=='D') {
295+
mv.visitInsn(F2D);
296+
}
297+
else if (targetDescriptor=='F') {
298+
// nop
299+
}
300+
else if (targetDescriptor=='J') {
301+
mv.visitInsn(F2L);
302+
}
303+
else if (targetDescriptor=='I') {
304+
mv.visitInsn(F2I);
305+
}
306+
else {
307+
throw new IllegalStateException("cannot get from "+stackTop+" to "+targetDescriptor);
308+
}
309+
}
310+
else if (stackTop=='D') {
311+
if (targetDescriptor=='D') {
312+
// nop
313+
}
314+
else if (targetDescriptor=='F') {
315+
mv.visitInsn(D2F);
316+
}
317+
else if (targetDescriptor=='J') {
318+
mv.visitInsn(D2L);
319+
}
320+
else if (targetDescriptor=='I') {
321+
mv.visitInsn(D2I);
322+
}
323+
else {
324+
throw new IllegalStateException("cannot get from "+stackDescriptor+" to "+targetDescriptor);
325+
}
326+
}
327+
}
328+
}
329+
330+
212331
/**
213332
* Create the JVM signature descriptor for a method. This consists of the descriptors
214333
* for the method parameters surrounded with parentheses, followed by the
@@ -836,5 +955,22 @@ public static void insertNewArrayCode(MethodVisitor mv, int size, String arrayty
836955
}
837956
}
838957

958+
/**
959+
* For use in mathematical operators, handles converting from a (possibly boxed) number on the stack to a primitive numeric type.
960+
* For example, from a Integer to a double, just need to call 'Number.doubleValue()' but from an int to a double, need to use
961+
* the bytecode 'i2d'.
962+
* @param mv the method visitor when instructions should be appended
963+
* @param stackDescriptor a descriptor of the operand on the stack
964+
* @param targetDescriptor a primitive type descriptor
965+
*/
966+
public static void insertNumericUnboxOrPrimitiveTypeCoercion(MethodVisitor mv,
967+
String stackDescriptor, char targetDecriptor) {
968+
if (!CodeFlow.isPrimitive(stackDescriptor)) {
969+
CodeFlow.insertUnboxNumberInsns(mv, targetDecriptor, stackDescriptor);
970+
} else {
971+
CodeFlow.insertAnyNecessaryTypeConversionBytecodes(mv, targetDecriptor, stackDescriptor);
972+
}
973+
}
974+
839975

840976
}

‎spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -59,15 +59,11 @@ public TypedValue getValueInternal(ExpressionState state) throws EvaluationExcep
5959
return new TypedValue(leftBigDecimal.divide(rightBigDecimal, scale, RoundingMode.HALF_EVEN));
6060
}
6161
else if (leftNumber instanceof Double || rightNumber instanceof Double) {
62-
if (leftNumber.getClass() == rightNumber.getClass()) {
63-
this.exitTypeDescriptor = "D";
64-
}
62+
this.exitTypeDescriptor = "D";
6563
return new TypedValue(leftNumber.doubleValue() / rightNumber.doubleValue());
6664
}
6765
else if (leftNumber instanceof Float || rightNumber instanceof Float) {
68-
if (leftNumber.getClass() == rightNumber.getClass()) {
69-
this.exitTypeDescriptor = "F";
70-
}
66+
this.exitTypeDescriptor = "F";
7167
return new TypedValue(leftNumber.floatValue() / rightNumber.floatValue());
7268
}
7369
else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger) {
@@ -76,15 +72,11 @@ else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger)
7672
return new TypedValue(leftBigInteger.divide(rightBigInteger));
7773
}
7874
else if (leftNumber instanceof Long || rightNumber instanceof Long) {
79-
if (leftNumber.getClass() == rightNumber.getClass()) {
80-
this.exitTypeDescriptor = "J";
81-
}
75+
this.exitTypeDescriptor = "J";
8276
return new TypedValue(leftNumber.longValue() / rightNumber.longValue());
8377
}
8478
else if (CodeFlow.isIntegerForNumericOp(leftNumber) || CodeFlow.isIntegerForNumericOp(rightNumber)) {
85-
if (leftNumber instanceof Integer && rightNumber instanceof Integer) {
86-
this.exitTypeDescriptor = "I";
87-
}
79+
this.exitTypeDescriptor = "I";
8880
return new TypedValue(leftNumber.intValue() / rightNumber.intValue());
8981
}
9082
else {
@@ -113,17 +105,13 @@ public boolean isCompilable() {
113105
public void generateCode(MethodVisitor mv, CodeFlow cf) {
114106
getLeftOperand().generateCode(mv, cf);
115107
String leftDesc = getLeftOperand().exitTypeDescriptor;
116-
if (!CodeFlow.isPrimitive(leftDesc)) {
117-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftDesc);
118-
}
108+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, leftDesc, this.exitTypeDescriptor.charAt(0));
119109
if (this.children.length > 1) {
120110
cf.enterCompilationScope();
121111
getRightOperand().generateCode(mv, cf);
122112
String rightDesc = getRightOperand().exitTypeDescriptor;
123113
cf.exitCompilationScope();
124-
if (!CodeFlow.isPrimitive(rightDesc)) {
125-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightDesc);
126-
}
114+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, rightDesc, this.exitTypeDescriptor.charAt(0));
127115
switch (this.exitTypeDescriptor.charAt(0)) {
128116
case 'I':
129117
mv.visitInsn(IDIV);

‎spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -108,15 +108,11 @@ else if (operand instanceof Byte) {
108108
return new TypedValue(leftBigDecimal.subtract(rightBigDecimal));
109109
}
110110
else if (leftNumber instanceof Double || rightNumber instanceof Double) {
111-
if (leftNumber.getClass() == rightNumber.getClass()) {
112-
this.exitTypeDescriptor = "D";
113-
}
111+
this.exitTypeDescriptor = "D";
114112
return new TypedValue(leftNumber.doubleValue() - rightNumber.doubleValue());
115113
}
116114
else if (leftNumber instanceof Float || rightNumber instanceof Float) {
117-
if (leftNumber.getClass() == rightNumber.getClass()) {
118-
this.exitTypeDescriptor = "F";
119-
}
115+
this.exitTypeDescriptor = "F";
120116
return new TypedValue(leftNumber.floatValue() - rightNumber.floatValue());
121117
}
122118
else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger) {
@@ -125,15 +121,11 @@ else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger)
125121
return new TypedValue(leftBigInteger.subtract(rightBigInteger));
126122
}
127123
else if (leftNumber instanceof Long || rightNumber instanceof Long) {
128-
if (leftNumber.getClass() == rightNumber.getClass()) {
129-
this.exitTypeDescriptor = "J";
130-
}
124+
this.exitTypeDescriptor = "J";
131125
return new TypedValue(leftNumber.longValue() - rightNumber.longValue());
132126
}
133127
else if (CodeFlow.isIntegerForNumericOp(leftNumber) || CodeFlow.isIntegerForNumericOp(rightNumber)) {
134-
if (leftNumber instanceof Integer && rightNumber instanceof Integer) {
135-
this.exitTypeDescriptor = "I";
136-
}
128+
this.exitTypeDescriptor = "I";
137129
return new TypedValue(leftNumber.intValue() - rightNumber.intValue());
138130
}
139131
else {
@@ -185,17 +177,13 @@ public boolean isCompilable() {
185177
public void generateCode(MethodVisitor mv, CodeFlow cf) {
186178
getLeftOperand().generateCode(mv, cf);
187179
String leftDesc = getLeftOperand().exitTypeDescriptor;
188-
if (!CodeFlow.isPrimitive(leftDesc)) {
189-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftDesc);
190-
}
180+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, leftDesc, this.exitTypeDescriptor.charAt(0));
191181
if (this.children.length > 1) {
192182
cf.enterCompilationScope();
193183
getRightOperand().generateCode(mv, cf);
194184
String rightDesc = getRightOperand().exitTypeDescriptor;
195185
cf.exitCompilationScope();
196-
if (!CodeFlow.isPrimitive(rightDesc)) {
197-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightDesc);
198-
}
186+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, rightDesc, this.exitTypeDescriptor.charAt(0));
199187
switch (this.exitTypeDescriptor.charAt(0)) {
200188
case 'I':
201189
mv.visitInsn(ISUB);

‎spring-expression/src/main/java/org/springframework/expression/spel/ast/OpModulus.java

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -57,15 +57,11 @@ public TypedValue getValueInternal(ExpressionState state) throws EvaluationExcep
5757
return new TypedValue(leftBigDecimal.remainder(rightBigDecimal));
5858
}
5959
else if (leftNumber instanceof Double || rightNumber instanceof Double) {
60-
if (leftNumber.getClass() == rightNumber.getClass()) {
61-
this.exitTypeDescriptor = "D";
62-
}
60+
this.exitTypeDescriptor = "D";
6361
return new TypedValue(leftNumber.doubleValue() % rightNumber.doubleValue());
6462
}
6563
else if (leftNumber instanceof Float || rightNumber instanceof Float) {
66-
if (leftNumber.getClass() == rightNumber.getClass()) {
67-
this.exitTypeDescriptor = "F";
68-
}
64+
this.exitTypeDescriptor = "F";
6965
return new TypedValue(leftNumber.floatValue() % rightNumber.floatValue());
7066
}
7167
else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger) {
@@ -74,15 +70,11 @@ else if (leftNumber instanceof BigInteger || rightNumber instanceof BigInteger)
7470
return new TypedValue(leftBigInteger.remainder(rightBigInteger));
7571
}
7672
else if (leftNumber instanceof Long || rightNumber instanceof Long) {
77-
if (leftNumber.getClass() == rightNumber.getClass()) {
78-
this.exitTypeDescriptor = "J";
79-
}
73+
this.exitTypeDescriptor = "J";
8074
return new TypedValue(leftNumber.longValue() % rightNumber.longValue());
8175
}
8276
else if (CodeFlow.isIntegerForNumericOp(leftNumber) || CodeFlow.isIntegerForNumericOp(rightNumber)) {
83-
if (leftNumber instanceof Integer && rightNumber instanceof Integer) {
84-
this.exitTypeDescriptor = "I";
85-
}
77+
this.exitTypeDescriptor = "I";
8678
return new TypedValue(leftNumber.intValue() % rightNumber.intValue());
8779
}
8880
else {
@@ -111,17 +103,13 @@ public boolean isCompilable() {
111103
public void generateCode(MethodVisitor mv, CodeFlow cf) {
112104
getLeftOperand().generateCode(mv, cf);
113105
String leftDesc = getLeftOperand().exitTypeDescriptor;
114-
if (!CodeFlow.isPrimitive(leftDesc)) {
115-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftDesc);
116-
}
106+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, leftDesc, this.exitTypeDescriptor.charAt(0));
117107
if (this.children.length > 1) {
118108
cf.enterCompilationScope();
119109
getRightOperand().generateCode(mv, cf);
120110
String rightDesc = getRightOperand().exitTypeDescriptor;
121111
cf.exitCompilationScope();
122-
if (!CodeFlow.isPrimitive(rightDesc)) {
123-
CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightDesc);
124-
}
112+
CodeFlow.insertNumericUnboxOrPrimitiveTypeCoercion(mv, rightDesc, this.exitTypeDescriptor.charAt(0));
125113
switch (this.exitTypeDescriptor.charAt(0)) {
126114
case 'I':
127115
mv.visitInsn(IREM);

0 commit comments

Comments
 (0)