From 42c152611c65f288f8f64f09216d2d40eeded14b Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Fri, 22 May 2020 11:34:38 -0400 Subject: [PATCH 01/11] Add unused type parameter to retain and release to prepare for closures --- src/compiler.ts | 95 ++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index e93e6e6848..678658257c 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -1181,7 +1181,7 @@ export class Compiler extends DiagnosticEmitter { ); } module.addGlobal(internalName, nativeType, true, this.makeZero(type)); - if (type.isManaged && !this.skippedAutoreleases.has(initExpr)) initExpr = this.makeRetain(initExpr); + if (type.isManaged && !this.skippedAutoreleases.has(initExpr)) initExpr = this.makeRetain(initExpr, type); this.currentBody.push( module.global_set(internalName, initExpr) ); @@ -1373,7 +1373,8 @@ export class Compiler extends DiagnosticEmitter { stmts.push( module.local_set(index, this.makeRetain( - module.local_get(index, type.toNativeType()) + module.local_get(index, type.toNativeType()), + type ) ) ); @@ -1674,7 +1675,7 @@ export class Compiler extends DiagnosticEmitter { module.local_get(0, nativeThisType), nativeValueType, instance.memoryOffset ); - if (type.isManaged) valueExpr = this.makeRetain(valueExpr); + if (type.isManaged) valueExpr = this.makeRetain(valueExpr, type); instance.getterRef = module.addFunction(instance.internalGetterName, nativeThisType, nativeValueType, null, valueExpr); if (instance.setterRef) { instance.set(CommonFlags.COMPILED); @@ -1711,9 +1712,9 @@ export class Compiler extends DiagnosticEmitter { ), module.block(null, [ module.drop( - this.makeRetain(module.local_get(1, nativeValueType)) + this.makeRetain(module.local_get(1, nativeValueType), type) ), - this.makeRelease(module.local_get(2, nativeValueType)) + this.makeRelease(module.local_get(2, nativeValueType), type) ]) ), module.local_get(1, nativeValueType) @@ -3099,7 +3100,7 @@ export class Compiler extends DiagnosticEmitter { module.local_set(local.index, initAutoreleaseSkipped ? initExpr - : this.makeRetain(initExpr) + : this.makeRetain(initExpr, type) ) ); } else { @@ -3497,7 +3498,7 @@ export class Compiler extends DiagnosticEmitter { // check if that worked, and if it didn't, keep the reference alive if (!this.skippedAutoreleases.has(expr)) { let index = this.tryUndoAutorelease(expr, flow); - if (index == -1) expr = this.makeRetain(expr); + if (index == -1) expr = this.makeRetain(expr, returnType); this.skippedAutoreleases.add(expr); } } @@ -5638,7 +5639,7 @@ export class Compiler extends DiagnosticEmitter { if (!leftAutoreleaseSkipped) { retainLeftInElse = true; } else { - rightExpr = this.makeRetain(rightExpr); + rightExpr = this.makeRetain(rightExpr, rightType); rightAutoreleaseSkipped = true; } } else if (!(constraints & Constraints.WILL_RETAIN)) { // otherwise keep right alive a little longer @@ -5649,7 +5650,8 @@ export class Compiler extends DiagnosticEmitter { if (leftAutoreleaseSkipped) { // left turned out to be true'ish and is dropped rightStmts.unshift( this.makeRelease( - module.local_get(temp.index, leftType.toNativeType()) + module.local_get(temp.index, leftType.toNativeType()), + leftType ) ); } @@ -5662,7 +5664,8 @@ export class Compiler extends DiagnosticEmitter { rightExpr, retainLeftInElse ? this.makeRetain( - module.local_get(temp.index, leftType.toNativeType()) + module.local_get(temp.index, leftType.toNativeType()), + leftType ) : module.local_get(temp.index, leftType.toNativeType()) ); @@ -5742,7 +5745,7 @@ export class Compiler extends DiagnosticEmitter { if (!leftAutoreleaseSkipped) { retainLeftInThen = true; } else { - rightExpr = this.makeRetain(rightExpr); + rightExpr = this.makeRetain(rightExpr, rightType); rightAutoreleaseSkipped = true; } } else if (!(constraints & Constraints.WILL_RETAIN)) { // otherwise keep right alive a little longer @@ -5755,7 +5758,8 @@ export class Compiler extends DiagnosticEmitter { // once implicit conversion with strings is performed and left is "", so: rightStmts.unshift( this.makeRelease( - module.local_get(temp.index, leftType.toNativeType()) + module.local_get(temp.index, leftType.toNativeType()), + rightType ) ); } @@ -5767,7 +5771,8 @@ export class Compiler extends DiagnosticEmitter { this.makeIsTrueish(leftExpr, leftType), retainLeftInThen ? this.makeRetain( - module.local_get(temp.index, leftType.toNativeType()) + module.local_get(temp.index, leftType.toNativeType()), + leftType ) : module.local_get(temp.index, leftType.toNativeType()), rightExpr @@ -6190,7 +6195,8 @@ export class Compiler extends DiagnosticEmitter { valueExpr = this.makeReplace( valueExpr, module.local_get(localIndex, type.toNativeType()), - alreadyRetained + alreadyRetained, + type ); if (tee) { // local = REPLACE(local, value) this.currentType = type; @@ -6202,7 +6208,7 @@ export class Compiler extends DiagnosticEmitter { } else { flow.unsetLocalFlag(localIndex, LocalFlags.CONDITIONALLY_RETAINED); flow.setLocalFlag(localIndex, LocalFlags.RETAINED); - if (!alreadyRetained) valueExpr = this.makeRetain(valueExpr); + if (!alreadyRetained) valueExpr = this.makeRetain(valueExpr, type); if (tee) { // local = __retain(value, local) this.currentType = type; return module.local_tee(localIndex, valueExpr); @@ -6246,7 +6252,8 @@ export class Compiler extends DiagnosticEmitter { this.makeReplace( valueExpr, module.global_get(global.internalName, nativeType), - alreadyRetained + alreadyRetained, + type ) ); if (tee) { // (global = REPLACE(global, value))), global @@ -6319,7 +6326,8 @@ export class Compiler extends DiagnosticEmitter { module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - alreadyRetained + alreadyRetained, + fieldType ), nativeFieldType, field.memoryOffset ), @@ -6336,7 +6344,8 @@ export class Compiler extends DiagnosticEmitter { module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - alreadyRetained + alreadyRetained, + fieldType ), nativeFieldType, field.memoryOffset ); @@ -6833,7 +6842,7 @@ export class Compiler extends DiagnosticEmitter { if (flow.isNonnull(paramExpr, paramType)) flow.setLocalFlag(argumentLocal.index, LocalFlags.NONNULL); // inlining is aware of skipped autoreleases: if (paramType.isManaged) { - if (!this.skippedAutoreleases.has(paramExpr)) paramExpr = this.makeRetain(paramExpr); + if (!this.skippedAutoreleases.has(paramExpr)) paramExpr = this.makeRetain(paramExpr, paramType); flow.setLocalFlag(argumentLocal.index, LocalFlags.RETAINED); } body.unshift( @@ -6877,7 +6886,7 @@ export class Compiler extends DiagnosticEmitter { if (flow.isNonnull(initExpr, initType)) flow.setLocalFlag(argumentLocal.index, LocalFlags.NONNULL); if (initType.isManaged) { flow.setLocalFlag(argumentLocal.index, LocalFlags.RETAINED); - if (!this.skippedAutoreleases.has(initExpr)) initExpr = this.makeRetain(initExpr); + if (!this.skippedAutoreleases.has(initExpr)) initExpr = this.makeRetain(initExpr, initType); } body.push( module.local_set(argumentLocal.index, initExpr) @@ -6901,7 +6910,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = returnType; if (returnType.isManaged) { if (immediatelyDropped) { - expr = this.makeRelease(expr); + expr = this.makeRelease(expr, returnType); this.currentType = Type.void; } } @@ -7238,14 +7247,14 @@ export class Compiler extends DiagnosticEmitter { // /** Makes a retain call, retaining the expression's value. */ - makeRetain(expr: ExpressionRef): ExpressionRef { + makeRetain(expr: ExpressionRef, type: Type | null = null): ExpressionRef { var retainInstance = this.program.retainInstance; this.compileFunction(retainInstance); return this.module.call(retainInstance.internalName, [ expr ], this.options.nativeSizeType); } /** Makes a release call, releasing the expression's value. Changes the current type to void.*/ - makeRelease(expr: ExpressionRef): ExpressionRef { + makeRelease(expr: ExpressionRef, type: Type | null = null): ExpressionRef { var releaseInstance = this.program.releaseInstance; this.compileFunction(releaseInstance); return this.module.call(releaseInstance.internalName, [ expr ], NativeType.None); @@ -7259,6 +7268,8 @@ export class Compiler extends DiagnosticEmitter { oldExpr: ExpressionRef, /** Whether the new value is already retained. */ alreadyRetained: bool = false, + /** The type shared between expressions. */ + type: Type ): ExpressionRef { var module = this.module; var flow = this.currentFlow; @@ -7269,7 +7280,7 @@ export class Compiler extends DiagnosticEmitter { let temp = flow.getTempLocal(this.options.usizeType, findUsedLocals(oldExpr)); let ret = module.block(null, [ module.local_set(temp.index, newExpr), - this.makeRelease(oldExpr), + this.makeRelease(oldExpr, type), module.local_get(temp.index, nativeSizeType) ], nativeSizeType); flow.freeTempLocal(temp); @@ -7290,9 +7301,9 @@ export class Compiler extends DiagnosticEmitter { ), module.block(null, [ module.local_set(temp1.index, - this.makeRetain(module.local_get(temp1.index, nativeSizeType)) + this.makeRetain(module.local_get(temp1.index, nativeSizeType), type) ), - this.makeRelease(module.local_get(temp2.index, nativeSizeType)) + this.makeRelease(module.local_get(temp2.index, nativeSizeType), type) ]) ), module.local_get(temp1.index, nativeSizeType) @@ -7399,7 +7410,7 @@ export class Compiler extends DiagnosticEmitter { // If it worked, autorelease in `outerFlow` instead ? this.makeAutorelease(expr, type, outerFlow) // If it didn't work, extend the lifetime into `outerFlow` - : this.makeAutorelease(this.makeRetain(expr), type, outerFlow); + : this.makeAutorelease(this.makeRetain(expr, type), type, outerFlow); } /** Performs any queued autoreleases in the specified flow. */ @@ -7429,7 +7440,8 @@ export class Compiler extends DiagnosticEmitter { if (finalize) flow.unsetLocalFlag(localIndex, LocalFlags.ANY_RETAINED); stmts.push( this.makeRelease( - module.local_get(localIndex, local.type.toNativeType()) + module.local_get(localIndex, local.type.toNativeType()), + local.type ) ); } @@ -7524,7 +7536,8 @@ export class Compiler extends DiagnosticEmitter { flow.unsetLocalFlag(localIndex, LocalFlags.ANY_RETAINED); stmts.push( this.makeRelease( - module.local_get(localIndex, local.type.toNativeType()) + module.local_get(localIndex, local.type.toNativeType()), + local.type ) ); } @@ -7661,7 +7674,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = returnType; if (returnType.isManaged) { if (immediatelyDropped) { - expr = this.makeRelease(expr); + expr = this.makeRelease(expr, returnType); this.currentType = Type.void; } else if (!skipAutorelease) { expr = this.makeAutorelease(expr, returnType); @@ -7687,7 +7700,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = returnType; if (returnType.isManaged) { if (immediatelyDropped) { - expr = this.makeRelease(expr); + expr = this.makeRelease(expr, returnType); this.currentType = Type.void; } else if (!skipAutorelease) { expr = this.makeAutorelease(expr, returnType); @@ -7800,7 +7813,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = returnType; if (returnType.isManaged) { if (immediatelyDropped) { - expr = this.makeRelease(expr); + expr = this.makeRelease(expr, returnType); this.currentType = Type.void; } else { expr = this.makeAutorelease(expr, returnType); @@ -8576,7 +8589,7 @@ export class Compiler extends DiagnosticEmitter { : module.i32(i64_low(bufferAddress)) ], expression); this.currentType = arrayType; - expr = this.makeRetain(expr); + expr = this.makeRetain(expr, elementType); if (arrayType.isManaged) { if (!(constraints & Constraints.WILL_RETAIN)) { expr = this.makeAutorelease(expr, arrayType); @@ -8795,7 +8808,7 @@ export class Compiler extends DiagnosticEmitter { if (isManaged) { // value = __retain(value) if (!this.skippedAutoreleases.has(valueExpr)) { - valueExpr = this.makeRetain(valueExpr); + valueExpr = this.makeRetain(valueExpr, elementType); } } // store(tempThis, value, immOffset) @@ -8921,7 +8934,7 @@ export class Compiler extends DiagnosticEmitter { let expr = this.compileExpression(values[i], fieldType, Constraints.CONV_IMPLICIT | Constraints.WILL_RETAIN); if (fieldType.isManaged && !this.skippedAutoreleases.has(expr)) { - expr = this.makeRetain(expr); + expr = this.makeRetain(expr, fieldType); } exprs.push( module.store( // TODO: handle setters as well @@ -9390,18 +9403,18 @@ export class Compiler extends DiagnosticEmitter { if (ifThenAutoreleaseSkipped != ifElseAutoreleaseSkipped) { // unify to both skipped if (!ifThenAutoreleaseSkipped) { - ifThenExpr = this.makeRetain(ifThenExpr); + ifThenExpr = this.makeRetain(ifThenExpr, ifThenType); ifThenAutoreleaseSkipped = true; } else { - ifElseExpr = this.makeRetain(ifElseExpr); + ifElseExpr = this.makeRetain(ifElseExpr, ifElseType); ifElseAutoreleaseSkipped = true; } } else if (!ifThenAutoreleaseSkipped && commonType.isManaged) { // keep alive a little longer if (constraints & Constraints.WILL_RETAIN) { // try to undo both let ifThenIndex = this.tryUndoAutorelease(ifThenExpr, ifThenFlow); - if (ifThenIndex == -1) ifThenExpr = this.makeRetain(ifThenExpr); + if (ifThenIndex == -1) ifThenExpr = this.makeRetain(ifThenExpr, ifThenType); let ifElseIndex = this.tryUndoAutorelease(ifElseExpr, ifElseFlow); - if (ifElseIndex == -1) ifElseExpr = this.makeRetain(ifElseExpr); + if (ifElseIndex == -1) ifElseExpr = this.makeRetain(ifElseExpr, ifElseType); ifThenAutoreleaseSkipped = true; ifElseAutoreleaseSkipped = true; } else { @@ -10472,7 +10485,7 @@ export class Compiler extends DiagnosticEmitter { : 1 + parameterIndex, // this is local 0 nativeFieldType ); - if (fieldType.isManaged) initExpr = this.makeRetain(initExpr); + if (fieldType.isManaged) initExpr = this.makeRetain(initExpr, fieldType); // fall back to use initializer if present } else if (initializerNode) { @@ -10480,7 +10493,7 @@ export class Compiler extends DiagnosticEmitter { Constraints.CONV_IMPLICIT | Constraints.WILL_RETAIN ); if (fieldType.isManaged && !this.skippedAutoreleases.has(initExpr)) { - initExpr = this.makeRetain(initExpr); + initExpr = this.makeRetain(initExpr, fieldType); } // otherwise initialize with zero From 6a087699799439254add3b250908398695e7cb34 Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Fri, 22 May 2020 11:44:02 -0400 Subject: [PATCH 02/11] Change argument order for makeReplace --- src/compiler.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index 678658257c..92d3bf161c 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6195,8 +6195,8 @@ export class Compiler extends DiagnosticEmitter { valueExpr = this.makeReplace( valueExpr, module.local_get(localIndex, type.toNativeType()), - alreadyRetained, - type + type, + alreadyRetained ); if (tee) { // local = REPLACE(local, value) this.currentType = type; @@ -6252,8 +6252,8 @@ export class Compiler extends DiagnosticEmitter { this.makeReplace( valueExpr, module.global_get(global.internalName, nativeType), - alreadyRetained, - type + type, + alreadyRetained ) ); if (tee) { // (global = REPLACE(global, value))), global @@ -6326,8 +6326,8 @@ export class Compiler extends DiagnosticEmitter { module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - alreadyRetained, - fieldType + fieldType, + alreadyRetained ), nativeFieldType, field.memoryOffset ), @@ -6344,8 +6344,8 @@ export class Compiler extends DiagnosticEmitter { module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - alreadyRetained, - fieldType + fieldType, + alreadyRetained ), nativeFieldType, field.memoryOffset ); @@ -7266,10 +7266,10 @@ export class Compiler extends DiagnosticEmitter { newExpr: ExpressionRef, /** Old value being replaced. */ oldExpr: ExpressionRef, - /** Whether the new value is already retained. */ - alreadyRetained: bool = false, /** The type shared between expressions. */ type: Type + /** Whether the new value is already retained. */ + alreadyRetained: bool = false, ): ExpressionRef { var module = this.module; var flow = this.currentFlow; From 549ed73c57e11d96adfc2904bdf28485d103f15f Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Fri, 22 May 2020 11:48:10 -0400 Subject: [PATCH 03/11] fix type --- src/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler.ts b/src/compiler.ts index 92d3bf161c..e1c41a9341 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -7267,7 +7267,7 @@ export class Compiler extends DiagnosticEmitter { /** Old value being replaced. */ oldExpr: ExpressionRef, /** The type shared between expressions. */ - type: Type + type: Type, /** Whether the new value is already retained. */ alreadyRetained: bool = false, ): ExpressionRef { From 105854513b7a5369918b836e7a9b543bbf025a65 Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 25 May 2020 17:03:45 -0400 Subject: [PATCH 04/11] Apply suggestions from code review Co-authored-by: Daniel Wirtz --- src/compiler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index e1c41a9341..bbb9df1d70 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -5759,7 +5759,7 @@ export class Compiler extends DiagnosticEmitter { rightStmts.unshift( this.makeRelease( module.local_get(temp.index, leftType.toNativeType()), - rightType + leftType ) ); } @@ -8589,7 +8589,7 @@ export class Compiler extends DiagnosticEmitter { : module.i32(i64_low(bufferAddress)) ], expression); this.currentType = arrayType; - expr = this.makeRetain(expr, elementType); + expr = this.makeRetain(expr, arrayType); if (arrayType.isManaged) { if (!(constraints & Constraints.WILL_RETAIN)) { expr = this.makeAutorelease(expr, arrayType); From a862df58450738c5cf7190784e148304c234b4da Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Mon, 25 May 2020 18:02:09 -0400 Subject: [PATCH 05/11] Require a type to always be passed to retain/release --- src/compiler.ts | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index bbb9df1d70..02b8b5ad62 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -1274,7 +1274,7 @@ export class Compiler extends DiagnosticEmitter { if (initInStart) { module.addGlobal(enumValue.internalName, NativeType.I32, true, module.i32(0)); this.currentBody.push( - this.makeGlobalAssignment(enumValue, initExpr, false) + this.makeGlobalAssignment(enumValue, initExpr, Type.i32, false) ); previousValueIsMut = true; } else { @@ -1512,7 +1512,8 @@ export class Compiler extends DiagnosticEmitter { ), module.local_set(thisLocal.index, this.makeRetain( - this.makeAllocation(classInstance) + this.makeAllocation(classInstance), + classInstance.type ) ) ) @@ -6026,7 +6027,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = tee ? global.type : Type.void; return module.unreachable(); } - return this.makeGlobalAssignment(global, valueExpr, tee); + return this.makeGlobalAssignment(global, valueExpr, global.type, tee); } case ElementKind.FIELD: { let fieldInstance = target; @@ -6048,6 +6049,7 @@ export class Compiler extends DiagnosticEmitter { assert(fieldParent.kind == ElementKind.CLASS); return this.makeFieldAssignment(fieldInstance, valueExpr, + valueType, this.compileExpression( assert(thisExpression), (fieldParent).type, @@ -6196,6 +6198,7 @@ export class Compiler extends DiagnosticEmitter { valueExpr, module.local_get(localIndex, type.toNativeType()), type, + valueType, alreadyRetained ); if (tee) { // local = REPLACE(local, value) @@ -6208,7 +6211,7 @@ export class Compiler extends DiagnosticEmitter { } else { flow.unsetLocalFlag(localIndex, LocalFlags.CONDITIONALLY_RETAINED); flow.setLocalFlag(localIndex, LocalFlags.RETAINED); - if (!alreadyRetained) valueExpr = this.makeRetain(valueExpr, type); + if (!alreadyRetained) valueExpr = this.makeRetain(valueExpr, valueType); if (tee) { // local = __retain(value, local) this.currentType = type; return module.local_tee(localIndex, valueExpr); @@ -6238,6 +6241,8 @@ export class Compiler extends DiagnosticEmitter { global: VariableLikeElement, /** The value to assign. */ valueExpr: ExpressionRef, + /** The type of the value to assign. */ + valueType: Type, /** Whether to tee the value. */ tee: bool ): ExpressionRef { @@ -6253,6 +6258,7 @@ export class Compiler extends DiagnosticEmitter { valueExpr, module.global_get(global.internalName, nativeType), type, + valueType, alreadyRetained ) ); @@ -6289,6 +6295,8 @@ export class Compiler extends DiagnosticEmitter { field: Field, /** The value to assign. */ valueExpr: ExpressionRef, + /** The type of the value to assign. */ + valueType: Type, /** The value of `this`. */ thisExpr: ExpressionRef, /** Whether to tee the value. */ @@ -6327,6 +6335,7 @@ export class Compiler extends DiagnosticEmitter { nativeFieldType, field.memoryOffset ), fieldType, + valueType, // TODOFIX alreadyRetained ), nativeFieldType, field.memoryOffset @@ -6345,6 +6354,7 @@ export class Compiler extends DiagnosticEmitter { nativeFieldType, field.memoryOffset ), fieldType, + fieldType, // TODOFIX alreadyRetained ), nativeFieldType, field.memoryOffset @@ -7247,14 +7257,14 @@ export class Compiler extends DiagnosticEmitter { // /** Makes a retain call, retaining the expression's value. */ - makeRetain(expr: ExpressionRef, type: Type | null = null): ExpressionRef { + makeRetain(expr: ExpressionRef, type: Type): ExpressionRef { var retainInstance = this.program.retainInstance; this.compileFunction(retainInstance); return this.module.call(retainInstance.internalName, [ expr ], this.options.nativeSizeType); } /** Makes a release call, releasing the expression's value. Changes the current type to void.*/ - makeRelease(expr: ExpressionRef, type: Type | null = null): ExpressionRef { + makeRelease(expr: ExpressionRef, type: Type): ExpressionRef { var releaseInstance = this.program.releaseInstance; this.compileFunction(releaseInstance); return this.module.call(releaseInstance.internalName, [ expr ], NativeType.None); @@ -7266,8 +7276,10 @@ export class Compiler extends DiagnosticEmitter { newExpr: ExpressionRef, /** Old value being replaced. */ oldExpr: ExpressionRef, - /** The type shared between expressions. */ - type: Type, + /** The type of the new expression. */ + newType: Type, + /** The type of the old expression. */ + oldType: Type, /** Whether the new value is already retained. */ alreadyRetained: bool = false, ): ExpressionRef { @@ -7280,7 +7292,7 @@ export class Compiler extends DiagnosticEmitter { let temp = flow.getTempLocal(this.options.usizeType, findUsedLocals(oldExpr)); let ret = module.block(null, [ module.local_set(temp.index, newExpr), - this.makeRelease(oldExpr, type), + this.makeRelease(oldExpr, oldType), module.local_get(temp.index, nativeSizeType) ], nativeSizeType); flow.freeTempLocal(temp); @@ -7301,9 +7313,9 @@ export class Compiler extends DiagnosticEmitter { ), module.block(null, [ module.local_set(temp1.index, - this.makeRetain(module.local_get(temp1.index, nativeSizeType), type) + this.makeRetain(module.local_get(temp1.index, nativeSizeType), newType) ), - this.makeRelease(module.local_get(temp2.index, nativeSizeType), type) + this.makeRelease(module.local_get(temp2.index, nativeSizeType), oldType) ]) ), module.local_get(temp1.index, nativeSizeType) @@ -8629,7 +8641,8 @@ export class Compiler extends DiagnosticEmitter { program.options.isWasm64 ? module.i64(0) : module.i32(0) - ], expression) + ], expression), + program.options.isWasm64 ? Type.i64 : Type.i32 ) ) ); @@ -8651,7 +8664,7 @@ export class Compiler extends DiagnosticEmitter { if (isManaged) { // value = __retain(value) if (!this.skippedAutoreleases.has(valueExpr)) { - valueExpr = this.makeRetain(valueExpr); + valueExpr = this.makeRetain(valueExpr, elementType); } } // store(tempData, value, immOffset) @@ -8761,7 +8774,8 @@ export class Compiler extends DiagnosticEmitter { isWasm64 ? module.i64(i64_low(bufferAddress), i64_high(bufferAddress)) : module.i32(i64_low(bufferAddress)) - ], expression) + ], expression), + isWasm64 ? Type.i64 : Type.i32 ); if (arrayType.isManaged) { if (constraints & Constraints.WILL_RETAIN) { @@ -8798,7 +8812,8 @@ export class Compiler extends DiagnosticEmitter { ? module.i64(bufferSize) : module.i32(bufferSize), module.i32(arrayInstance.id) - ], expression) + ], expression), + isWasm64 ? Type.i64 : Type.i32 ) ) ); @@ -9149,7 +9164,7 @@ export class Compiler extends DiagnosticEmitter { // return this // } var allocExpr = this.makeAllocation(classInstance); - if (classInstance.type.isManaged) allocExpr = this.makeRetain(allocExpr); + if (classInstance.type.isManaged) allocExpr = this.makeRetain(allocExpr, classInstance.type); stmts.push( module.if( module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32, From 1fd8748b9be50e6b0d1d655613c5c6f60d10267c Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Tue, 26 May 2020 10:31:46 -0400 Subject: [PATCH 06/11] Apply PR comment changes --- src/compiler.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index 02b8b5ad62..86ad2c70ac 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6196,8 +6196,8 @@ export class Compiler extends DiagnosticEmitter { if (flow.isAnyLocalFlag(localIndex, LocalFlags.ANY_RETAINED)) { valueExpr = this.makeReplace( valueExpr, - module.local_get(localIndex, type.toNativeType()), type, + module.local_get(localIndex, type.toNativeType()), valueType, alreadyRetained ); @@ -6256,8 +6256,8 @@ export class Compiler extends DiagnosticEmitter { valueExpr = module.global_set(global.internalName, this.makeReplace( valueExpr, - module.global_get(global.internalName, nativeType), type, + module.global_get(global.internalName, nativeType), valueType, alreadyRetained ) @@ -6330,11 +6330,11 @@ export class Compiler extends DiagnosticEmitter { module.local_tee(tempThis.index, thisExpr), this.makeReplace( module.local_tee(tempValue.index, valueExpr), + fieldType, module.load(fieldType.byteSize, fieldType.is(TypeFlags.SIGNED), module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - fieldType, valueType, // TODOFIX alreadyRetained ), @@ -6349,12 +6349,12 @@ export class Compiler extends DiagnosticEmitter { module.local_tee(tempThis.index, thisExpr), this.makeReplace( valueExpr, + fieldType, module.load(fieldType.byteSize, fieldType.is(TypeFlags.SIGNED), module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - fieldType, - fieldType, // TODOFIX + valueType, alreadyRetained ), nativeFieldType, field.memoryOffset @@ -7274,10 +7274,10 @@ export class Compiler extends DiagnosticEmitter { makeReplace( /** New value being assigned. */ newExpr: ExpressionRef, - /** Old value being replaced. */ - oldExpr: ExpressionRef, /** The type of the new expression. */ newType: Type, + /** Old value being replaced. */ + oldExpr: ExpressionRef, /** The type of the old expression. */ oldType: Type, /** Whether the new value is already retained. */ @@ -8642,7 +8642,7 @@ export class Compiler extends DiagnosticEmitter { ? module.i64(0) : module.i32(0) ], expression), - program.options.isWasm64 ? Type.i64 : Type.i32 + arrayType ) ) ); @@ -8775,7 +8775,7 @@ export class Compiler extends DiagnosticEmitter { ? module.i64(i64_low(bufferAddress), i64_high(bufferAddress)) : module.i32(i64_low(bufferAddress)) ], expression), - isWasm64 ? Type.i64 : Type.i32 + program.arrayBufferInstance.type ); if (arrayType.isManaged) { if (constraints & Constraints.WILL_RETAIN) { @@ -8813,7 +8813,7 @@ export class Compiler extends DiagnosticEmitter { : module.i32(bufferSize), module.i32(arrayInstance.id) ], expression), - isWasm64 ? Type.i64 : Type.i32 + program.arrayBufferInstance.type ) ) ); From 310b51cb6903d4b527911815bce5321acb76d763 Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Tue, 26 May 2020 10:33:38 -0400 Subject: [PATCH 07/11] remove old todo comment --- src/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler.ts b/src/compiler.ts index 86ad2c70ac..caeb714b77 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6335,7 +6335,7 @@ export class Compiler extends DiagnosticEmitter { module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - valueType, // TODOFIX + valueType, alreadyRetained ), nativeFieldType, field.memoryOffset From 2ed0693519d6c402dcd37d01af7e7cc9e5db66c9 Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Tue, 26 May 2020 10:37:37 -0400 Subject: [PATCH 08/11] fix ordering of types --- src/compiler.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index caeb714b77..da84200acd 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6196,9 +6196,9 @@ export class Compiler extends DiagnosticEmitter { if (flow.isAnyLocalFlag(localIndex, LocalFlags.ANY_RETAINED)) { valueExpr = this.makeReplace( valueExpr, - type, - module.local_get(localIndex, type.toNativeType()), valueType, + module.local_get(localIndex, type.toNativeType()), + type, alreadyRetained ); if (tee) { // local = REPLACE(local, value) @@ -6256,9 +6256,9 @@ export class Compiler extends DiagnosticEmitter { valueExpr = module.global_set(global.internalName, this.makeReplace( valueExpr, - type, - module.global_get(global.internalName, nativeType), valueType, + module.global_get(global.internalName, nativeType), + type, alreadyRetained ) ); From 633a9a3954c14bfdc98e89358a8a57b5819f216f Mon Sep 17 00:00:00 2001 From: Duncan Date: Tue, 26 May 2020 10:35:11 -0400 Subject: [PATCH 09/11] Apply suggestions from code review Co-authored-by: Daniel Wirtz --- src/compiler.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index da84200acd..a9acd27867 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6027,7 +6027,7 @@ export class Compiler extends DiagnosticEmitter { this.currentType = tee ? global.type : Type.void; return module.unreachable(); } - return this.makeGlobalAssignment(global, valueExpr, global.type, tee); + return this.makeGlobalAssignment(global, valueExpr, valueType, tee); } case ElementKind.FIELD: { let fieldInstance = target; @@ -9164,7 +9164,8 @@ export class Compiler extends DiagnosticEmitter { // return this // } var allocExpr = this.makeAllocation(classInstance); - if (classInstance.type.isManaged) allocExpr = this.makeRetain(allocExpr, classInstance.type); + let classType = classInstance.type; + if (classType.isManaged) allocExpr = this.makeRetain(allocExpr, classType); stmts.push( module.if( module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32, From d61b7ea9b008070941e75777f780eb35eac4f2b5 Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Tue, 26 May 2020 10:40:10 -0400 Subject: [PATCH 10/11] fix make replace ordering --- src/compiler.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler.ts b/src/compiler.ts index a9acd27867..44e948d49e 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6330,12 +6330,12 @@ export class Compiler extends DiagnosticEmitter { module.local_tee(tempThis.index, thisExpr), this.makeReplace( module.local_tee(tempValue.index, valueExpr), - fieldType, + valueType, module.load(fieldType.byteSize, fieldType.is(TypeFlags.SIGNED), module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - valueType, + fieldType, alreadyRetained ), nativeFieldType, field.memoryOffset @@ -6349,12 +6349,12 @@ export class Compiler extends DiagnosticEmitter { module.local_tee(tempThis.index, thisExpr), this.makeReplace( valueExpr, - fieldType, + valueType, module.load(fieldType.byteSize, fieldType.is(TypeFlags.SIGNED), module.local_get(tempThis.index, nativeThisType), nativeFieldType, field.memoryOffset ), - valueType, + fieldType, alreadyRetained ), nativeFieldType, field.memoryOffset From 4d7dc3052f71a48673ebd1c436d06c72c1546b31 Mon Sep 17 00:00:00 2001 From: Duncan Uszkay Date: Tue, 26 May 2020 11:25:32 -0400 Subject: [PATCH 11/11] lint fixes --- src/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler.ts b/src/compiler.ts index 44e948d49e..a36d0a5bf9 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -9164,7 +9164,7 @@ export class Compiler extends DiagnosticEmitter { // return this // } var allocExpr = this.makeAllocation(classInstance); - let classType = classInstance.type; + var classType = classInstance.type; if (classType.isManaged) allocExpr = this.makeRetain(allocExpr, classType); stmts.push( module.if(