Skip to content

Commit

Permalink
Keep track of array types in OptimizationBasicInterpreter
Browse files Browse the repository at this point in the history
Merging array types with different element types, for example
`[Lj/l/String;` and `[Lj/l/Object;`, now produces `[Lj/l/Object;`
(instead of `Lj/l/Object;`), which allows for more precise tracking of
null values because we assume that AALOAD on a non-array typed value is
possible only if that value is null.

 #KT-54802 Fixed
  • Loading branch information
udalov committed Nov 7, 2022
1 parent 7deaab9 commit e7f77e9
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.kotlin.codegen.AsmUtil;
import org.jetbrains.kotlin.resolve.jvm.AsmTypes;
import org.jetbrains.org.objectweb.asm.Handle;
import org.jetbrains.org.objectweb.asm.Opcodes;
import org.jetbrains.org.objectweb.asm.Type;
Expand Down Expand Up @@ -152,13 +153,6 @@ public BasicValue binaryOperation(
@NotNull BasicValue value1,
@NotNull BasicValue value2
) throws AnalyzerException {
if (insn.getOpcode() == Opcodes.AALOAD) {
Type arrayType = value1.getType();
if (arrayType != null && arrayType.getSort() == Type.ARRAY) {
return new StrictBasicValue(AsmUtil.correctElementType(arrayType));
}
}

switch (insn.getOpcode()) {
case IALOAD:
case BALOAD:
Expand Down Expand Up @@ -204,7 +198,13 @@ public BasicValue binaryOperation(
case DREM:
return StrictBasicValue.DOUBLE_VALUE;
case AALOAD:
return StrictBasicValue.NULL_VALUE;
Type arrayType = value1.getType();
if (arrayType != null && arrayType.getSort() == Type.ARRAY) {
return new StrictBasicValue(AsmUtil.correctElementType(arrayType));
}
else {
return StrictBasicValue.NULL_VALUE;
}
case LCMP:
case FCMPL:
case FCMPG:
Expand Down Expand Up @@ -359,13 +359,11 @@ public BasicValue merge(
return StrictBasicValue.UNINITIALIZED_VALUE;
}

// if merge of two references then `lub` is java/lang/Object
// arrays also are BasicValues with reference type's
if (isReference(v) && isReference(w)) {
if (v == NULL_VALUE) return newValue(w.getType());
if (w == NULL_VALUE) return newValue(v.getType());

return StrictBasicValue.REFERENCE_VALUE;
return mergeReferenceTypes(w.getType(), v.getType());
}

// if merge of something can be stored in int var (int, char, boolean, byte, character)
Expand All @@ -380,4 +378,34 @@ public BasicValue merge(
private static boolean isReference(@NotNull BasicValue v) {
return v.getType().getSort() == Type.OBJECT || v.getType().getSort() == Type.ARRAY;
}

// Merge reference types, keeping track of array dimensions.
// See also org.jetbrains.org.objectweb.asm.Frame.merge.
private BasicValue mergeReferenceTypes(@NotNull Type a, @NotNull Type b) {
// Find out the minimal array dimension of both types.
int arrayDimensions = 0;
while (a.getSort() == Type.ARRAY && b.getSort() == Type.ARRAY) {
a = AsmUtil.correctElementType(a);
b = AsmUtil.correctElementType(b);
arrayDimensions++;
}
// Either of the two types is not an array -> result is j/l/Object.
if (arrayDimensions == 0) return REFERENCE_VALUE;

// Both of the types are arrays, and element type of one or both of them is primitive ->
// result is array of j/l/Object with one fewer dimension. E.g.
// merge([I, [Lj/l/Object;) = Lj/l/Object;
// merge([I, [S) = Lj/l/Object;
// merge([[I, [[Lj/l/Object;) = [Lj/l/Object;
if (AsmUtil.isPrimitive(a) || AsmUtil.isPrimitive(b)) {
arrayDimensions--;
}

// Result is array of j/l/Object with the computed dimension.
StringBuilder result = new StringBuilder();
while (arrayDimensions-- > 0) result.append("[");
result.append(AsmTypes.OBJECT_TYPE.getDescriptor());
return newValue(Type.getType(result.toString()));
}

}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions compiler/testData/codegen/box/casts/kt54707.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// IGNORE_BACKEND: JVM
// IGNORE_LIGHT_ANALYSIS

fun box(): String =
g(arrayOf("O"))

Expand All @@ -12,4 +9,3 @@ inline fun <T> Array<out T>.f(lambda: (T) -> T): T =

inline fun <reified T> Array<out T>?.orEmpty0(): Array<out T> =
this ?: (arrayOfNulls<T>(0) as Array<T>)

14 changes: 14 additions & 0 deletions compiler/testData/codegen/box/casts/kt54802.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class K {
val x: String = "OK"
}

inline fun <T> Array<out T>.ifEmpty(body: () -> Array<out T>): Array<out T> =
if (size == 0) body() else this

inline fun <T> Array<out T>.f(p: (T) -> String): String =
p(this[0])

fun box(): String =
emptyArray<K>()
.ifEmpty { arrayOf(K()) }
.f(K::x)

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e7f77e9

Please sign in to comment.