azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix for duplicated fields in regenerated lambda

Browse Source
This commit is contained in:
Mikhael Bogdanov
2014-03-18 12:20:18 +04:00
parent 961cb7d349
commit 001945cd12
13 changed files with 132 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/jet/codegen/inline/CapturedParamInfo.java
+22 -6
View File
@@ -19,7 +19,6 @@ package org.jetbrains.jet.codegen.inline;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.asm4.Type;
import org.jetbrains.jet.codegen.StackValue;
import org.jetbrains.jet.lang.resolve.java.AsmTypeConstants;
public class CapturedParamInfo extends ParameterInfo {
@@ -34,17 +33,31 @@ public class CapturedParamInfo extends ParameterInfo {
private int shift = 0;
private final String newFieldName;
public CapturedParamInfo(@NotNull CapturedParamDesc desc, boolean skipped, int index, int remapIndex) {
super(desc.getType(), skipped, index, remapIndex);
this.desc = desc;
this(desc, desc.getFieldName(), skipped, index, remapIndex);
}
public CapturedParamInfo(@NotNull CapturedParamDesc desc, boolean skipped, int index, StackValue remapIndex) {
public CapturedParamInfo(@NotNull CapturedParamDesc desc, @NotNull String newFieldName, boolean skipped, int index, int remapIndex) {
super(desc.getType(), skipped, index, remapIndex);
this.desc = desc;
this.newFieldName = newFieldName;
}
public String getFieldName() {
public CapturedParamInfo(@NotNull CapturedParamDesc desc, @NotNull String newFieldName, boolean skipped, int index, StackValue remapIndex) {
super(desc.getType(), skipped, index, remapIndex);
this.desc = desc;
this.newFieldName = newFieldName;
}
@NotNull
public String getNewFieldName() {
return newFieldName;
}
@NotNull
public String getOriginalFieldName() {
return desc.getFieldName();
}
@@ -57,16 +70,19 @@ public class CapturedParamInfo extends ParameterInfo {
this.shift = shift;
}
@NotNull
public CapturedParamInfo newIndex(int newIndex) {
return clone(newIndex, getRemapValue());
}
@NotNull
public CapturedParamInfo clone(int newIndex, StackValue newRamapIndex) {
CapturedParamInfo capturedParamInfo = new CapturedParamInfo(desc, isSkipped, newIndex, newRamapIndex);
CapturedParamInfo capturedParamInfo = new CapturedParamInfo(desc, newFieldName, isSkipped, newIndex, newRamapIndex);
capturedParamInfo.setLambda(lambda);
return capturedParamInfo;
}
@NotNull
public String getContainingLambdaName() {
return desc.getContainingLambda().getType().getInternalName();
}
compiler/backend/src/org/jetbrains/jet/codegen/inline/FieldRemapper.java
+4 -10
View File
@@ -41,13 +41,7 @@ public class FieldRemapper {
params = methodParams;
}
public void addCapturedFields(LambdaInfo lambdaInfo, ParametersBuilder builder) {
for (CapturedParamInfo info : lambdaInfo.getCapturedVars()) {
builder.addCapturedParam(info, info);
}
}
public boolean canProcess(@NotNull String fieldOwner) {
public boolean canProcess(@NotNull String fieldOwner, boolean forTransformation) {
return fieldOwner.equals(getLambdaInternalName());
}
@@ -79,7 +73,7 @@ public class FieldRemapper {
if (transformed == null) {
//if parent couldn't transform
FieldInsnNode insnNode = (FieldInsnNode) capturedFieldAccess.get(currentInstruction);
if (canProcess(insnNode.owner)) {
if (canProcess(insnNode.owner, true)) {
insnNode.name = "$$$" + insnNode.name;
insnNode.setOpcode(Opcodes.GETSTATIC);
@@ -102,9 +96,9 @@ public class FieldRemapper {
}
@Nullable
public CapturedParamInfo findField(@NotNull FieldInsnNode fieldInsnNode, @NotNull Collection<CapturedParamInfo> captured) {
protected CapturedParamInfo findField(@NotNull FieldInsnNode fieldInsnNode, @NotNull Collection<CapturedParamInfo> captured) {
for (CapturedParamInfo valueDescriptor : captured) {
if (valueDescriptor.getFieldName().equals(fieldInsnNode.name) && fieldInsnNode.owner.equals(valueDescriptor.getContainingLambdaName())) {
if (valueDescriptor.getOriginalFieldName().equals(fieldInsnNode.name) && fieldInsnNode.owner.equals(valueDescriptor.getContainingLambdaName())) {
return valueDescriptor;
}
}
compiler/backend/src/org/jetbrains/jet/codegen/inline/InlineCodegen.java
+1 -1
View File
@@ -217,7 +217,7 @@ public class InlineCodegen implements ParentCodegenAware, CallGenerator {
VarRemapper.ParamRemapper remapper = new VarRemapper.ParamRemapper(parameters, initialFrameSize);
return inliner.doInline(codegen.v, remapper, new FieldRemapper(null, null, parameters));
return inliner.doInline(codegen.v, remapper);
}
private void generateClosuresBodies() {
compiler/backend/src/org/jetbrains/jet/codegen/inline/InlinedLambdaRemapper.java
+2 -14
View File
@@ -34,13 +34,10 @@ public class InlinedLambdaRemapper extends FieldRemapper {
@Override
public void addCapturedFields(
LambdaInfo lambdaInfo, ParametersBuilder builder
) {
parent.addCapturedFields(lambdaInfo, builder);
public boolean canProcess(@NotNull String fieldOwner, boolean forTransformation) {
return forTransformation ? super.canProcess(fieldOwner, forTransformation) : false;
}
@Override
@Nullable
public CapturedParamInfo findField(
@@ -50,13 +47,4 @@ public class InlinedLambdaRemapper extends FieldRemapper {
return parent.findField(fieldInsnNode, captured);
}
@Override
public FieldRemapper getParent() {
return parent.getParent();
}
@Override
public boolean isRoot() {
return parent.isRoot();
}
}
compiler/backend/src/org/jetbrains/jet/codegen/inline/LambdaInfo.java
+4 -2
View File
@@ -138,7 +138,7 @@ public class LambdaInfo implements CapturedParamOwner {
return Arrays.asList(types);
}
public Parameters addAllParameters(@NotNull FieldRemapper remapper) {
public Parameters addAllParameters() {
ParametersBuilder builder = ParametersBuilder.newBuilder();
//add skipped this cause inlined lambda doesn't have it
builder.addThis(AsmTypeConstants.OBJECT_TYPE, true).setLambda(this);
@@ -149,7 +149,9 @@ public class LambdaInfo implements CapturedParamOwner {
builder.addNextParameter(type, false, null);
}
remapper.addCapturedFields(this, builder);
for (CapturedParamInfo info : getCapturedVars()) {
builder.addCapturedParam(info, info.getOriginalFieldName());
}
return builder.buildParameters();
}
compiler/backend/src/org/jetbrains/jet/codegen/inline/LambdaTransformer.java
+25 -10
View File
@@ -32,10 +32,7 @@ import org.jetbrains.jet.codegen.state.GenerationState;
import org.jetbrains.jet.codegen.state.JetTypeMapper;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
import static org.jetbrains.asm4.Opcodes.ASM4;
import static org.jetbrains.asm4.Opcodes.V1_6;
@@ -64,6 +61,8 @@ public class LambdaTransformer {
private String[] interfaces;
private final boolean isSameModule;
private Map<String, List<String>> fieldNames = new HashMap<String, List<String>>();
public LambdaTransformer(String lambdaInternalName, InliningContext inliningContext, boolean isSameModule, Type newLambdaType) {
this.isSameModule = isSameModule;
this.state = inliningContext.state;
@@ -133,7 +132,7 @@ public class LambdaTransformer {
MethodInliner inliner = new MethodInliner(invoke, parameters, inliningContext.subInline(inliningContext.nameGenerator.subGenerator("lambda")),
remapper, isSameModule, "Transformer for " + invocation.getOwnerInternalName());
InlineResult result = inliner.doInline(invokeVisitor, new VarRemapper.ParamRemapper(parameters, 0), remapper, false);
InlineResult result = inliner.doInline(invokeVisitor, new VarRemapper.ParamRemapper(parameters, 0), false);
invokeVisitor.visitMaxs(-1, -1);
generateConstructorAndFields(classBuilder, builder, invocation);
@@ -163,7 +162,7 @@ public class LambdaTransformer {
List<Pair<String, Type>> newConstructorSignature = new ArrayList<Pair<String, Type>>();
for (CapturedParamInfo capturedParamInfo : infos) {
if (capturedParamInfo.getLambda() == null) { //not inlined
newConstructorSignature.add(new Pair<String, Type>(capturedParamInfo.getFieldName(), capturedParamInfo.getType()));
newConstructorSignature.add(new Pair<String, Type>(capturedParamInfo.getNewFieldName(), capturedParamInfo.getType()));
}
}
@@ -224,6 +223,8 @@ public class LambdaTransformer {
info.setLambda(lambdaInfo);
capturedLambdas.add(lambdaInfo);
}
addUniqueField(info.getOriginalFieldName());
}
cur = cur.getNext();
}
@@ -234,11 +235,11 @@ public class LambdaTransformer {
List<CapturedParamInfo> allRecapturedParameters = new ArrayList<CapturedParamInfo>();
for (LambdaInfo info : capturedLambdas) {
for (CapturedParamInfo var : info.getCapturedVars()) {
CapturedParamInfo recapturedParamInfo = builder.addCapturedParam(getNewFieldName(var.getFieldName()), var.getType(), var.isSkipped, var, info);
CapturedParamInfo recapturedParamInfo = builder.addCapturedParam(var, getNewFieldName(var.getOriginalFieldName()));
StackValue composed = StackValue.composed(StackValue.local(0, oldLambdaType),
StackValue.field(var.getType(),
oldLambdaType, /*TODO owner type*/
getNewFieldName(var.getFieldName()), false)
recapturedParamInfo.getNewFieldName(), false)
);
recapturedParamInfo.setRemapValue(composed);
allRecapturedParameters.add(var);
@@ -283,11 +284,25 @@ public class LambdaTransformer {
return methodNode[0];
}
public static String getNewFieldName(String oldName) {
@NotNull
public String getNewFieldName(@NotNull String oldName) {
if (oldName.equals("this$0")) {
//"this$0" couldn't clash and we should keep this name invariant for further transformations
return oldName;
}
return oldName + "$inlined";
return addUniqueField(oldName + "$inlined");
}
@NotNull
private String addUniqueField(@NotNull String name) {
List<String> existNames = fieldNames.get(name);
if (existNames == null) {
existNames = new LinkedList<String>();
fieldNames.put(name, existNames);
}
String suffix = existNames.isEmpty() ? "" : "$" + existNames.size();
String newName = name + suffix;
existNames.add(newName);
return newName;
}
}
compiler/backend/src/org/jetbrains/jet/codegen/inline/MethodInliner.java
+11 -14
View File
@@ -72,19 +72,19 @@ public class MethodInliner {
}
public InlineResult doInline(MethodVisitor adapter, VarRemapper.ParamRemapper remapper, FieldRemapper capturedRemapper) {
return doInline(adapter, remapper, capturedRemapper, true);
public InlineResult doInline(MethodVisitor adapter, VarRemapper.ParamRemapper remapper) {
return doInline(adapter, remapper, true);
}
public InlineResult doInline(
MethodVisitor adapter,
VarRemapper.ParamRemapper remapper,
FieldRemapper capturedRemapper, boolean remapReturn
boolean remapReturn
) {
//analyze body
MethodNode transformedNode = markPlacesForInlineAndRemoveInlinable(node);
transformedNode = doInline(transformedNode, capturedRemapper);
transformedNode = doInline(transformedNode);
removeClosureAssertions(transformedNode);
transformedNode.instructions.resetLabels();
@@ -102,7 +102,7 @@ public class MethodInliner {
return result;
}
private MethodNode doInline(MethodNode node, final FieldRemapper capturedRemapper) {
private MethodNode doInline(MethodNode node) {
final Deque<InvokeCall> currentInvokes = new LinkedList<InvokeCall>(invokeCalls);
@@ -129,7 +129,7 @@ public class MethodInliner {
inliningContext.subInline(inliningContext.nameGenerator, currentTypeMapping).classRegeneration(),
isSameModule, newLambdaType);
InlineResult transformResult = transformer.doTransform(invocation, capturedRemapper);
InlineResult transformResult = transformer.doTransform(invocation, nodeRemapper);
result.addAllClassesToRemove(transformResult);
if (inliningContext.isInliningLambda) {
@@ -159,22 +159,19 @@ public class MethodInliner {
int valueParamShift = getNextLocalIndex();//NB: don't inline cause it changes
putStackValuesIntoLocals(info.getParamsWithoutCapturedValOrVar(), valueParamShift, this, desc);
Parameters lambdaParameters = info.addAllParameters(capturedRemapper);
Parameters lambdaParameters = info.addAllParameters();
InlinedLambdaRemapper newCapturedRemapper =
new InlinedLambdaRemapper(info.getLambdaClassType().getInternalName(), capturedRemapper, lambdaParameters);
FieldRemapper fieldRemapper =
new FieldRemapper(info.getLambdaClassType().getInternalName(), capturedRemapper, lambdaParameters);
new InlinedLambdaRemapper(info.getLambdaClassType().getInternalName(), nodeRemapper, lambdaParameters);
setInlining(true);
MethodInliner inliner = new MethodInliner(info.getNode(), lambdaParameters,
inliningContext.subInlineLambda(info),
fieldRemapper, true /*cause all calls in same module as lambda*/,
newCapturedRemapper, true /*cause all calls in same module as lambda*/,
"Lambda inlining " + info.getLambdaClassType().getInternalName());
VarRemapper.ParamRemapper remapper = new VarRemapper.ParamRemapper(lambdaParameters, valueParamShift);
InlineResult lambdaResult = inliner.doInline(this.mv, remapper, newCapturedRemapper);//TODO add skipped this and receiver
InlineResult lambdaResult = inliner.doInline(this.mv, remapper);//TODO add skipped this and receiver
result.addAllClassesToRemove(lambdaResult);
//return value boxing/unboxing
@@ -188,7 +185,7 @@ public class MethodInliner {
if (invocation.shouldRegenerate()) {
//put additional captured parameters on stack
for (CapturedParamInfo capturedParamInfo : invocation.getAllRecapturedParameters()) {
visitFieldInsn(Opcodes.GETSTATIC, capturedParamInfo.getContainingLambdaName(), "$$$" + capturedParamInfo.getFieldName(), capturedParamInfo.getType().getDescriptor());
visitFieldInsn(Opcodes.GETSTATIC, capturedParamInfo.getContainingLambdaName(), "$$$" + capturedParamInfo.getOriginalFieldName(), capturedParamInfo.getType().getDescriptor());
}
super.visitMethodInsn(opcode, invocation.getNewLambdaType().getInternalName(), name, invocation.getNewConstructorDescriptor());
invocation = null;
compiler/backend/src/org/jetbrains/jet/codegen/inline/ParametersBuilder.java
+8 -5
View File
@@ -42,20 +42,23 @@ public class ParametersBuilder {
return info;
}
public ParametersBuilder addNextParameter(Type type, boolean skipped, @Nullable ParameterInfo original) {
@NotNull
public ParametersBuilder addNextParameter(@NotNull Type type, boolean skipped, @Nullable ParameterInfo original) {
addParameter(new ParameterInfo(type, skipped, nextIndex, original != null ? original.getIndex() : -1));
return this;
}
@NotNull
public CapturedParamInfo addCapturedParam(
@NotNull CapturedParamInfo param,
@Nullable CapturedParamInfo originalField
@NotNull CapturedParamInfo original,
@NotNull String newFieldName
) {
CapturedParamInfo info = new CapturedParamInfo(param.desc, param.isSkipped, nextCaptured, originalField != null ? originalField.getIndex() : -1);
info.setLambda(param.getLambda());
CapturedParamInfo info = new CapturedParamInfo(original.desc, newFieldName, original.isSkipped, nextCaptured, original.getIndex());
info.setLambda(original.getLambda());
return addCapturedParameter(info);
}
@NotNull
public CapturedParamInfo addCapturedParam(
@NotNull String fieldName,
@NotNull Type type,
compiler/backend/src/org/jetbrains/jet/codegen/inline/RegeneratedLambdaFieldRemapper.java
+7 -48
View File
@@ -24,7 +24,6 @@ import org.jetbrains.asm4.tree.FieldInsnNode;
import org.jetbrains.jet.codegen.StackValue;
import java.util.Collection;
import java.util.List;
import java.util.Map;
public class RegeneratedLambdaFieldRemapper extends FieldRemapper {
@@ -52,35 +51,8 @@ public class RegeneratedLambdaFieldRemapper extends FieldRemapper {
}
@Override
public void addCapturedFields(LambdaInfo lambdaInfo, ParametersBuilder builder) {
if (canProcess(lambdaInfo.getLambdaClassType().getInternalName())) {
List<CapturedParamInfo> captured = parameters.getCaptured();
for (CapturedParamInfo originalField : lambdaInfo.getCapturedVars()) {
CapturedParamInfo foundField = null;
for (CapturedParamInfo capturedParamInfo : captured) {
if (capturedParamInfo.getContainingLambdaName().equals(originalField.getContainingLambdaName())) {
if (capturedParamInfo.getFieldName().equals(LambdaTransformer.getNewFieldName(originalField.getFieldName()))) {
foundField = originalField;
break;
}
}
}
if (foundField == null) {
throw new IllegalStateException("Captured parameter should exists in outer context: " + originalField.getFieldName());
}
builder.addCapturedParam(foundField, foundField);
}
} else {
//in case when inlining lambda into another one inside inline function
parent.addCapturedFields(lambdaInfo, builder);
}
}
@Override
public boolean canProcess(@NotNull String fieldOwner) {
return super.canProcess(fieldOwner) || isRecapturedLambdaType(fieldOwner);
public boolean canProcess(@NotNull String fieldOwner, boolean forTransformation) {
return super.canProcess(fieldOwner, forTransformation) || isRecapturedLambdaType(fieldOwner);
}
private boolean isRecapturedLambdaType(String owner) {
@@ -90,27 +62,17 @@ public class RegeneratedLambdaFieldRemapper extends FieldRemapper {
@Nullable
@Override
public CapturedParamInfo findField(@NotNull FieldInsnNode fieldInsnNode, @NotNull Collection<CapturedParamInfo> captured) {
boolean searchInParent = !canProcess(fieldInsnNode.owner);
boolean searchInParent = !canProcess(fieldInsnNode.owner, false);
if (searchInParent) {
return parent.findField(fieldInsnNode);
} else if (isRecapturedLambdaType(fieldInsnNode.owner)) {
LambdaInfo info = recapturedLambdas.get(fieldInsnNode.owner);
return super.findField(fieldInsnNode, info.getCapturedVars());
}
else {
return super.findField(fieldInsnNode, captured);
} else {
return findFieldInMyCaptured(fieldInsnNode);
}
}
@Nullable
public CapturedParamInfo findFieldInMyCaptured(@NotNull FieldInsnNode fieldInsnNode) {
if (isRecapturedLambdaType(fieldInsnNode.owner)) {
LambdaInfo info = recapturedLambdas.get(fieldInsnNode.owner);
return super.findField(fieldInsnNode, info.getCapturedVars());
}
else {
return super.findField(fieldInsnNode, parameters.getCaptured());
}
return super.findField(fieldInsnNode, parameters.getCaptured());
}
@Nullable
@@ -129,14 +91,11 @@ public class RegeneratedLambdaFieldRemapper extends FieldRemapper {
}
}
String newName = field.getContainingLambdaName().equals(getLambdaInternalName())
? field.getFieldName()
: LambdaTransformer.getNewFieldName(field.getFieldName());
StackValue result =
StackValue.composed(prefix == null ? StackValue.local(0, Type.getObjectType(getLambdaInternalName())) : prefix,
StackValue.field(field.getType(),
Type.getObjectType(newOwnerType), /*TODO owner type*/
newName, false)
field.getNewFieldName(), false)
);
return searchInParent ? parent.getFieldForInline(node, result) : result;
compiler/testData/codegen/boxInline/sameCaptured/1.kt
+22
View File
@@ -0,0 +1,22 @@
import test.*
fun testSameCaptured() : String {
var result = 0;
result = doWork({result+=1; result}, {result += 11; result})
return if (result == 12) "OK" else "fail ${result}"
}
inline fun testSameCaptured(lambdaWithResultCaptured: () -> Unit) : String {
var result = 1;
result = doWork({result+=11; lambdaWithResultCaptured(); result})
return if (result == 12) "OK" else "fail ${result}"
}
fun box(): String {
if (testSameCaptured() != "OK") return "test1 : ${testSameCaptured()}"
var result = 0;
if (testSameCaptured{result += 1111} != "OK") return "test2 : ${testSameCaptured{result = 1111}}"
return "OK"
}
compiler/testData/codegen/boxInline/sameCaptured/2.kt
+16
View File
@@ -0,0 +1,16 @@
package test
inline fun <R> doWork(job: ()-> R) : R {
val k = 10;
return notInline({k; job()})
}
inline fun <R> doWork(job: ()-> R, job2: () -> R) : R {
val k = 10;
return notInline({k; job(); job2()})
}
fun <R> notInline(job: ()-> R) : R {
return job()
}
compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxInlineCodegenTestGenerated.java
+5
View File
@@ -176,6 +176,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxCodegenT
doTestMultiFile("compiler/testData/codegen/boxInline/rootConstructor");
}
@TestMetadata("sameCaptured")
public void testSameCaptured() throws Exception {
doTestMultiFile("compiler/testData/codegen/boxInline/sameCaptured");
}
@TestMetadata("severalClosures")
public void testSeveralClosures() throws Exception {
doTestMultiFile("compiler/testData/codegen/boxInline/severalClosures");
compiler/tests/org/jetbrains/jet/jvm/compiler/CompileKotlinAgainstInlineKotlinTestGenerated.java
+5
View File
@@ -176,6 +176,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi
doBoxTest("compiler/testData/codegen/boxInline/rootConstructor");
}
@TestMetadata("sameCaptured")
public void testSameCaptured() throws Exception {
doBoxTest("compiler/testData/codegen/boxInline/sameCaptured");
}
@TestMetadata("severalClosures")
public void testSeveralClosures() throws Exception {
doBoxTest("compiler/testData/codegen/boxInline/severalClosures");