diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index d974f6ca75..259215d052 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -28,15 +28,28 @@ import org.springframework.asm.Opcodes; import org.springframework.util.Assert; /** - * Manages the class being generated by the compilation process. It records - * intermediate compilation state as the bytecode is generated. It also includes - * various bytecode generation helper functions. + * Manages the class being generated by the compilation process. + * + *

Records intermediate compilation state as the bytecode is generated. + * Also includes various bytecode generation helper functions. * * @author Andy Clement + * @author Juergen Hoeller * @since 4.1 */ public class CodeFlow implements Opcodes { + /** + * Name of the class being generated. Typically used when generating code + * that accesses freshly generated fields on the generated type. + */ + private final String className; + + /** + * The current class being generated. + */ + private final ClassWriter classWriter; + /** * Record the type of what is on top of the bytecode stack (i.e. the type of the * output from the previous expression component). New scopes are used to evaluate @@ -45,17 +58,12 @@ public class CodeFlow implements Opcodes { */ private final Stack> compilationScopes; - /** - * The current class being generated - */ - private ClassWriter cw; - /** * As SpEL ast nodes are called to generate code for the main evaluation method * they can register to add a field to this class. Any registered FieldAdders * will be called after the main evaluation function has finished being generated. */ - private List fieldAdders = null; + private List fieldAdders; /** * As SpEL ast nodes are called to generate code for the main evaluation method @@ -63,13 +71,7 @@ public class CodeFlow implements Opcodes { * registered ClinitAdders will be called after the main evaluation function * has finished being generated. */ - private List clinitAdders = null; - - /** - * Name of the class being generated. Typically used when generating code - * that accesses freshly generated fields on the generated type. - */ - private String clazzName; + private List clinitAdders; /** * When code generation requires holding a value in a class level field, this @@ -83,13 +85,20 @@ public class CodeFlow implements Opcodes { */ private int nextFreeVariableId = 1; - public CodeFlow(String clazzName, ClassWriter cw) { - this.compilationScopes = new Stack<>(); - this.compilationScopes.add(new ArrayList<>()); - this.cw = cw; - this.clazzName = clazzName; + + /** + * Construct a new {@code CodeFlow} for the given class. + * @param className the name of the class + * @param classWriter the corresponding ASM {@code ClassWriter} + */ + public CodeFlow(String className, ClassWriter classWriter) { + this.className = className; + this.classWriter = classWriter; + this.compilationScopes = new Stack>(); + this.compilationScopes.add(new ArrayList()); } + /** * Push the byte code to load the target (i.e. what was passed as the first argument * to CompiledExpression.getValue(target, context)) @@ -103,6 +112,7 @@ public class CodeFlow implements Opcodes { * Push the bytecode to load the EvaluationContext (the second parameter passed to * the compiled expression method). * @param mv the visitor into which the load instruction should be inserted + * @since 4.3.4 */ public void loadEvaluationContext(MethodVisitor mv) { mv.visitVarInsn(ALOAD, 2); @@ -164,18 +174,18 @@ public class CodeFlow implements Opcodes { public void finish() { if (this.fieldAdders != null) { for (FieldAdder fieldAdder : this.fieldAdders) { - fieldAdder.generateField(cw,this); + fieldAdder.generateField(this.classWriter, this); } } if (this.clinitAdders != null) { - MethodVisitor mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "", "()V", null, null); + MethodVisitor mv = this.classWriter.visitMethod(ACC_PUBLIC | ACC_STATIC, "", "()V", null, null); mv.visitCode(); - this.nextFreeVariableId = 0; // To 0 because there is no 'this' in a clinit + this.nextFreeVariableId = 0; // to 0 because there is no 'this' in a clinit for (ClinitAdder clinitAdder : this.clinitAdders) { clinitAdder.generateCode(mv, this); } mv.visitInsn(RETURN); - mv.visitMaxs(0,0); // not supplied due to COMPUTE_MAXS + mv.visitMaxs(0,0); // not supplied due to COMPUTE_MAXS mv.visitEnd(); } } @@ -213,13 +223,13 @@ public class CodeFlow implements Opcodes { } public String getClassName() { - return this.clazzName; + return this.className; } /** * Insert any necessary cast and value call to convert from a boxed type to a - * primitive value + * primitive value. * @param mv the method visitor into which instructions should be inserted * @param ch the primitive type desired as output * @param stackDescriptor the descriptor of the type on top of the stack diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index f0add1519a..9aef460db0 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -252,7 +252,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals(false, expression.getValue()); assertCanCompile(expression); assertEquals(false, expression.getValue()); - + // double slot left operand - should get boxed, return false expression = parse("3.0d instanceof T(Integer)"); assertEquals(false, expression.getValue()); @@ -264,7 +264,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals(true, expression.getValue()); assertCanCompile(expression); assertEquals(true, expression.getValue()); - + // Only when the right hand operand is a direct type reference // will it be compilable. StandardEvaluationContext ctx = new StandardEvaluationContext(); @@ -279,7 +279,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals(false, expression.getValue()); assertCanCompile(expression); assertEquals(false, expression.getValue()); - + expression = parse("3 instanceof T(long)"); assertEquals(false, expression.getValue()); assertCanCompile(expression); @@ -1624,120 +1624,78 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @Test public void opNe_SPR14863() throws Exception { - // First part is from the test case specified in the bug report + SpelParserConfiguration configuration = + new SpelParserConfiguration(SpelCompilerMode.MIXED, ClassLoader.getSystemClassLoader()); + SpelExpressionParser parser = new SpelExpressionParser(configuration); + Expression expression = parser.parseExpression("data['my-key'] != 'my-value'"); + Map data = new HashMap<>(); - data.put("my-key", new String("my-value")); - StandardEvaluationContext context = new StandardEvaluationContext(new MyContext(data)); - SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.MIXED, - ClassLoader.getSystemClassLoader()); - SpelExpressionParser parser = new SpelExpressionParser(configuration); - Expression expression = parser.parseExpression("data['my-key'] != 'my-value'"); - assertFalse(expression.getValue(context, Boolean.class)); - assertCanCompile(expression); - ((SpelExpression) expression).compileExpression(); - assertFalse(expression.getValue(context, Boolean.class)); - - List ls = new ArrayList(); - ls.add(new String("foo")); - context = new StandardEvaluationContext(ls); - expression = parse("get(0) != 'foo'"); - assertFalse(expression.getValue(context, Boolean.class)); - assertCanCompile(expression); - assertFalse(expression.getValue(context, Boolean.class)); - - ls.remove(0); - ls.add("goo"); - assertTrue(expression.getValue(context, Boolean.class)); - } - - @Test - public void opEq_SPR14863() throws Exception { - // Exercise the comparator invocation code that runs in - // equalityCheck() (called from interpreted and compiled code) - expression = parser.parseExpression("#aa==#bb"); - StandardEvaluationContext sec = new StandardEvaluationContext(); - Apple aa = new Apple(1); - Apple bb = new Apple(2); - sec.setVariable("aa",aa); - sec.setVariable("bb",bb); - boolean b = expression.getValue(sec, Boolean.class); - // Verify what the expression caused aa to be compared to - assertEquals(bb,aa.gotComparedTo); - assertFalse(b); - bb.setValue(1); - b = expression.getValue(sec, Boolean.class); - assertEquals(bb,aa.gotComparedTo); - assertTrue(b); - - assertCanCompile(expression); - - // Similar test with compiled expression - aa = new Apple(99); - bb = new Apple(100); - sec.setVariable("aa",aa); - sec.setVariable("bb",bb); - b = expression.getValue(sec, Boolean.class); - assertFalse(b); - assertEquals(bb,aa.gotComparedTo); - bb.setValue(99); - b = expression.getValue(sec, Boolean.class); - assertTrue(b); - assertEquals(bb,aa.gotComparedTo); - - - List ls = new ArrayList(); - ls.add(new String("foo")); - StandardEvaluationContext context = new StandardEvaluationContext(ls); - expression = parse("get(0) == 'foo'"); - assertTrue(expression.getValue(context, Boolean.class)); - assertCanCompile(expression); - assertTrue(expression.getValue(context, Boolean.class)); - - ls.remove(0); - ls.add("goo"); - assertFalse(expression.getValue(context, Boolean.class)); - } - - public static class Apple implements Comparable { - public Object gotComparedTo = null; - public int i; + data.put("my-key", new String("my-value")); + StandardEvaluationContext context = new StandardEvaluationContext(new MyContext(data)); + assertFalse(expression.getValue(context, Boolean.class)); + assertCanCompile(expression); + ((SpelExpression) expression).compileExpression(); + assertFalse(expression.getValue(context, Boolean.class)); - public Apple(int i) { - this.i = i; - } - - public void setValue(int i) { - this.i = i; - } + List ls = new ArrayList(); + ls.add(new String("foo")); + context = new StandardEvaluationContext(ls); + expression = parse("get(0) != 'foo'"); + assertFalse(expression.getValue(context, Boolean.class)); + assertCanCompile(expression); + assertFalse(expression.getValue(context, Boolean.class)); - @Override - public int compareTo(Apple that) { - this.gotComparedTo = that; - if (this.ithat.i) { - return +1; - } - else { - return 0; - } - } - + ls.remove(0); + ls.add("goo"); + assertTrue(expression.getValue(context, Boolean.class)); } - - // For opNe_SPR14863 - public static class MyContext { - - private final Map data; - public MyContext(Map data) { - this.data = data; - } + @Test + public void opEq_SPR14863() throws Exception { + // Exercise the comparator invocation code that runs in + // equalityCheck() (called from interpreted and compiled code) + expression = parser.parseExpression("#aa==#bb"); + StandardEvaluationContext sec = new StandardEvaluationContext(); + Apple aa = new Apple(1); + Apple bb = new Apple(2); + sec.setVariable("aa",aa); + sec.setVariable("bb",bb); + boolean b = expression.getValue(sec, Boolean.class); + // Verify what the expression caused aa to be compared to + assertEquals(bb,aa.gotComparedTo); + assertFalse(b); + bb.setValue(1); + b = expression.getValue(sec, Boolean.class); + assertEquals(bb,aa.gotComparedTo); + assertTrue(b); + + assertCanCompile(expression); + + // Similar test with compiled expression + aa = new Apple(99); + bb = new Apple(100); + sec.setVariable("aa",aa); + sec.setVariable("bb",bb); + b = expression.getValue(sec, Boolean.class); + assertFalse(b); + assertEquals(bb,aa.gotComparedTo); + bb.setValue(99); + b = expression.getValue(sec, Boolean.class); + assertTrue(b); + assertEquals(bb,aa.gotComparedTo); + + + List ls = new ArrayList(); + ls.add(new String("foo")); + StandardEvaluationContext context = new StandardEvaluationContext(ls); + expression = parse("get(0) == 'foo'"); + assertTrue(expression.getValue(context, Boolean.class)); + assertCanCompile(expression); + assertTrue(expression.getValue(context, Boolean.class)); - public Map getData() { - return data; - } + ls.remove(0); + ls.add("goo"); + assertFalse(expression.getValue(context, Boolean.class)); } @Test @@ -2733,7 +2691,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { checkCalc(p,"payload.valueI*payload.valueBB20",2400); } - @Test public void opModulus_mixedNumberTypes() throws Exception { PayloadX p = new PayloadX(); @@ -4430,10 +4387,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { // // time it interpreted // long stime = System.currentTimeMillis(); - // for (int i = 0;i<100000;i++) { + // for (int i = 0; i < 100000; i++) { // v = expression.getValue(ctx,holder); // } - // System.out.println((System.currentTimeMillis()-stime)); + // System.out.println((System.currentTimeMillis() - stime)); assertCanCompile(expression); v = expression.getValue(ctx,holder); @@ -4441,10 +4398,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { // // time it compiled // stime = System.currentTimeMillis(); - // for (int i = 0;i<100000;i++) { + // for (int i = 0; i < 100000; i++) { // v = expression.getValue(ctx,holder); // } - // System.out.println((System.currentTimeMillis()-stime)); + // System.out.println((System.currentTimeMillis() - stime)); } @Test @@ -5660,4 +5617,48 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } } + + public static class Apple implements Comparable { + + public Object gotComparedTo = null; + public int i; + + public Apple(int i) { + this.i = i; + } + + public void setValue(int i) { + this.i = i; + } + + @Override + public int compareTo(Apple that) { + this.gotComparedTo = that; + if (this.i < that.i) { + return -1; + } + else if (this.i > that.i) { + return +1; + } + else { + return 0; + } + } + } + + + // For opNe_SPR14863 + public static class MyContext { + + private final Map data; + + public MyContext(Map data) { + this.data = data; + } + + public Map getData() { + return data; + } + } + }