Skip to content

Commit

Permalink
Simplify compilation of array indexing in SpEL's Indexer
Browse files Browse the repository at this point in the history
  • Loading branch information
sbrannen committed Mar 23, 2024
1 parent 7edd4e8 commit 2a1abb5
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ private enum IndexedType {ARRAY, LIST, MAP, STRING, OBJECT}
@Nullable
private IndexedType indexedType;

@Nullable
private volatile String arrayTypeDescriptor;

// These fields are used when the indexer is being used as a property read accessor.
// If the name and target type match these cached values then the cachedReadAccessor
// is used to read the property. If they do not match, the correct accessor is
Expand Down Expand Up @@ -212,7 +215,7 @@ else if (target instanceof Collection<?> collection) {
@Override
public boolean isCompilable() {
if (this.indexedType == IndexedType.ARRAY) {
return (this.exitTypeDescriptor != null);
return (this.exitTypeDescriptor != null && this.arrayTypeDescriptor != null);
}
SpelNodeImpl index = this.children[0];
if (this.indexedType == IndexedType.LIST) {
Expand All @@ -233,6 +236,7 @@ else if (this.indexedType == IndexedType.OBJECT) {

@Override
public void generateCode(MethodVisitor mv, CodeFlow cf) {
String exitTypeDescriptor = this.exitTypeDescriptor;
String descriptor = cf.lastDescriptor();
if (descriptor == null) {
// Stack is empty, should use context object
Expand All @@ -242,48 +246,19 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
SpelNodeImpl index = this.children[0];

if (this.indexedType == IndexedType.ARRAY) {
String exitTypeDescriptor = this.exitTypeDescriptor;
Assert.state(exitTypeDescriptor != null, "Array not compilable without descriptor");
int insn = switch (exitTypeDescriptor) {
case "D" -> {
mv.visitTypeInsn(CHECKCAST, "[D");
yield DALOAD;
}
case "F" -> {
mv.visitTypeInsn(CHECKCAST, "[F");
yield FALOAD;
}
case "J" -> {
mv.visitTypeInsn(CHECKCAST, "[J");
yield LALOAD;
}
case "I" -> {
mv.visitTypeInsn(CHECKCAST, "[I");
yield IALOAD;
}
case "S" -> {
mv.visitTypeInsn(CHECKCAST, "[S");
yield SALOAD;
}
case "B" -> {
mv.visitTypeInsn(CHECKCAST, "[B");
// byte and boolean arrays are both loaded via BALOAD
yield BALOAD;
}
case "Z" -> {
mv.visitTypeInsn(CHECKCAST, "[Z");
// byte and boolean arrays are both loaded via BALOAD
yield BALOAD;
}
case "C" -> {
mv.visitTypeInsn(CHECKCAST, "[C");
yield CALOAD;
}
default -> {
mv.visitTypeInsn(CHECKCAST, "["+ exitTypeDescriptor +
(CodeFlow.isPrimitiveArray(exitTypeDescriptor) ? "" : ";"));
yield AALOAD;
}
String arrayTypeDescriptor = this.arrayTypeDescriptor;
Assert.state(exitTypeDescriptor != null && arrayTypeDescriptor != null,
"Array not compilable without descriptors");
CodeFlow.insertCheckCast(mv, arrayTypeDescriptor);
int insn = switch (arrayTypeDescriptor) {
case "[D" -> DALOAD;
case "[F" -> FALOAD;
case "[J" -> LALOAD;
case "[I" -> IALOAD;
case "[S" -> SALOAD;
case "[B", "[Z" -> BALOAD; // byte[] & boolean[] are both loaded via BALOAD
case "[C" -> CALOAD;
default -> AALOAD;
};

cf.enterCompilationScope();
Expand Down Expand Up @@ -329,7 +304,7 @@ else if (this.indexedType == IndexedType.OBJECT) {
compilablePropertyAccessor.generateCode(propertyName, mv, cf);
}

cf.pushDescriptor(this.exitTypeDescriptor);
cf.pushDescriptor(exitTypeDescriptor);
}

@Override
Expand Down Expand Up @@ -394,55 +369,64 @@ private Object accessArrayElement(Object ctx, int idx) throws SpelEvaluationExce
boolean[] array = (boolean[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "Z";
this.arrayTypeDescriptor = "[Z";
return array[idx];
}
else if (arrayComponentType == byte.class) {
byte[] array = (byte[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "B";
this.arrayTypeDescriptor = "[B";
return array[idx];
}
else if (arrayComponentType == char.class) {
char[] array = (char[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "C";
this.arrayTypeDescriptor = "[C";
return array[idx];
}
else if (arrayComponentType == double.class) {
double[] array = (double[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "D";
this.arrayTypeDescriptor = "[D";
return array[idx];
}
else if (arrayComponentType == float.class) {
float[] array = (float[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "F";
this.arrayTypeDescriptor = "[F";
return array[idx];
}
else if (arrayComponentType == int.class) {
int[] array = (int[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "I";
this.arrayTypeDescriptor = "[I";
return array[idx];
}
else if (arrayComponentType == long.class) {
long[] array = (long[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "J";
this.arrayTypeDescriptor = "[J";
return array[idx];
}
else if (arrayComponentType == short.class) {
short[] array = (short[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "S";
this.arrayTypeDescriptor = "[S";
return array[idx];
}
else {
Object[] array = (Object[]) ctx;
checkAccess(array.length, idx);
Object retValue = array[idx];
this.exitTypeDescriptor = CodeFlow.toDescriptor(arrayComponentType);
this.arrayTypeDescriptor = CodeFlow.toDescriptor(array.getClass());
return retValue;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,18 @@ void indexIntoMapOfPrimitiveIntArrayWithCompilableMapAccessor() {
assertThat(getAst().getExitDescriptor()).isEqualTo("I");
}

@Test
void indexIntoSetCannotBeCompiled() {
Set<Integer> set = Set.of(42);

expression = parser.parseExpression("[0]");

assertThat(expression.getValue(set)).isEqualTo(42);
assertCannotCompile(expression);
assertThat(expression.getValue(set)).isEqualTo(42);
assertThat(getAst().getExitDescriptor()).isNull();
}

@Test
void indexIntoStringCannotBeCompiled() {
String text = "enigma";
Expand Down

0 comments on commit 2a1abb5

Please sign in to comment.