Skip to content

Commit 0c02d01

Browse files
dcodeIOdcodeIO
authored
Rework implicit allocation in constructors (#1255)
1 parent b5cb60f commit 0c02d01

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+7643
-4472
lines changed

‎src/ast.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,7 @@ export enum DecoratorKind {
13041304
OPERATOR_PREFIX,
13051305
OPERATOR_POSTFIX,
13061306
UNMANAGED,
1307-
SEALED,
1307+
FINAL,
13081308
INLINE,
13091309
EXTERNAL,
13101310
BUILTIN,
@@ -1316,7 +1316,6 @@ export namespace DecoratorKind {
13161316

13171317
/** Returns the kind of the specified decorator name node. Defaults to {@link DecoratorKind.CUSTOM}. */
13181318
export function fromNode(nameNode: Expression): DecoratorKind {
1319-
// @global, @inline, @operator, @sealed, @unmanaged
13201319
if (nameNode.kind == NodeKind.IDENTIFIER) {
13211320
let nameStr = (<IdentifierExpression>nameNode).text;
13221321
assert(nameStr.length);
@@ -1329,6 +1328,10 @@ export namespace DecoratorKind {
13291328
if (nameStr == "external") return DecoratorKind.EXTERNAL;
13301329
break;
13311330
}
1331+
case CharCode.f: {
1332+
if (nameStr == "final") return DecoratorKind.FINAL;
1333+
break;
1334+
}
13321335
case CharCode.g: {
13331336
if (nameStr == "global") return DecoratorKind.GLOBAL;
13341337
break;
@@ -1345,10 +1348,6 @@ export namespace DecoratorKind {
13451348
if (nameStr == "operator") return DecoratorKind.OPERATOR;
13461349
break;
13471350
}
1348-
case CharCode.s: {
1349-
if (nameStr == "sealed") return DecoratorKind.SEALED;
1350-
break;
1351-
}
13521351
case CharCode.u: {
13531352
if (nameStr == "unmanaged") return DecoratorKind.UNMANAGED;
13541353
if (nameStr == "unsafe") return DecoratorKind.UNSAFE;
@@ -1363,7 +1362,6 @@ export namespace DecoratorKind {
13631362
assert(nameStr.length);
13641363
let propStr = propertyAccessNode.property.text;
13651364
assert(propStr.length);
1366-
// @operator.binary, @operator.prefix, @operator.postfix
13671365
if (nameStr == "operator") {
13681366
switch (propStr.charCodeAt(0)) {
13691367
case CharCode.b: {

‎src/compiler.ts

Lines changed: 79 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,13 @@ export class Compiler extends DiagnosticEmitter {
13071307
let index = 0;
13081308
let thisType = signature.thisType;
13091309
if (thisType) {
1310-
// No need to retain `this` as it can't be reassigned and thus can't become prematurely released
1310+
// In normal instance functions, `this` is effectively a constant
1311+
// retained elsewhere so does not need to be retained.
1312+
if (instance.is(CommonFlags.CONSTRUCTOR)) {
1313+
// Constructors, however, can allocate their own memory, and as such
1314+
// must refcount the allocation in case something else is `return`ed.
1315+
flow.setLocalFlag(index, LocalFlags.RETAINED);
1316+
}
13111317
++index;
13121318
}
13131319
let parameterTypes = signature.parameterTypes;
@@ -1343,7 +1349,7 @@ export class Compiler extends DiagnosticEmitter {
13431349
signature.nativeParams,
13441350
signature.nativeResults,
13451351
typesToNativeTypes(instance.additionalLocals),
1346-
module.flatten(stmts, instance.signature.returnType.toNativeType())
1352+
body
13471353
);
13481354

13491355
// imported function
@@ -1392,6 +1398,9 @@ export class Compiler extends DiagnosticEmitter {
13921398
var bodyNode = assert(instance.prototype.bodyNode);
13931399
var returnType = instance.signature.returnType;
13941400
var flow = this.currentFlow;
1401+
var thisLocal = instance.is(CommonFlags.INSTANCE)
1402+
? assert(flow.lookupLocal(CommonNames.this_))
1403+
: null;
13951404

13961405
// compile statements
13971406
if (bodyNode.kind == NodeKind.BLOCK) {
@@ -1432,39 +1441,66 @@ export class Compiler extends DiagnosticEmitter {
14321441
}
14331442
}
14341443

1435-
// make constructors return their instance pointer
1444+
// Make constructors return their instance pointer, and prepend a conditional
1445+
// allocation if any code path accesses `this`.
14361446
if (instance.is(CommonFlags.CONSTRUCTOR)) {
14371447
let nativeSizeType = this.options.nativeSizeType;
14381448
assert(instance.is(CommonFlags.INSTANCE));
1449+
thisLocal = assert(thisLocal);
14391450
let parent = assert(instance.parent);
14401451
assert(parent.kind == ElementKind.CLASS);
14411452
let classInstance = <Class>parent;
14421453

1443-
if (!flow.is(FlowFlags.TERMINATES)) {
1444-
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
1445-
1446-
// if `this` wasn't accessed before, allocate if necessary and initialize `this`
1447-
if (!flow.is(FlowFlags.ALLOCATES)) {
1448-
// {
1449-
// if (!this) this = <ALLOC>
1450-
// this.a = X
1451-
// this.b = Y
1452-
// }
1453-
stmts.push(
1454-
module.if(
1455-
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
1456-
module.local_get(thisLocal.index, nativeSizeType)
1457-
),
1458-
module.local_set(thisLocal.index,
1459-
this.makeRetain(
1460-
this.makeAllocation(classInstance)
1461-
),
1454+
if (flow.isAny(FlowFlags.ACCESSES_THIS | FlowFlags.CONDITIONALLY_ACCESSES_THIS) || !flow.is(FlowFlags.TERMINATES)) {
1455+
// Allocate `this` if not a super call, and initialize fields
1456+
let allocStmts = new Array<ExpressionRef>();
1457+
allocStmts.push(
1458+
module.if(
1459+
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
1460+
module.local_get(thisLocal.index, nativeSizeType)
1461+
),
1462+
module.local_set(thisLocal.index,
1463+
this.makeRetain(
1464+
this.makeAllocation(classInstance)
14621465
)
14631466
)
1467+
)
1468+
);
1469+
this.makeFieldInitializationInConstructor(classInstance, allocStmts);
1470+
if (flow.isInline) {
1471+
let firstStmt = stmts[0]; // `this` alias assignment
1472+
assert(getExpressionId(firstStmt) == ExpressionId.LocalSet);
1473+
assert(getLocalSetIndex(firstStmt) == thisLocal.index);
1474+
allocStmts.unshift(firstStmt);
1475+
stmts[0] = module.flatten(allocStmts, NativeType.None);
1476+
} else {
1477+
stmts.unshift(
1478+
module.flatten(allocStmts, NativeType.None)
14641479
);
1465-
this.makeFieldInitializationInConstructor(classInstance, stmts);
14661480
}
1467-
this.performAutoreleases(flow, stmts); // `this` is excluded anyway
1481+
1482+
// Just prepended allocation is dropped when returning non-'this'
1483+
if (flow.is(FlowFlags.MAY_RETURN_NONTHIS)) {
1484+
this.pedantic(
1485+
DiagnosticCode.Explicitly_returning_constructor_drops_this_allocation,
1486+
instance.identifierNode.range
1487+
);
1488+
}
1489+
}
1490+
1491+
// Returning something else than 'this' would break 'super()' calls
1492+
if (flow.is(FlowFlags.MAY_RETURN_NONTHIS) && !classInstance.hasDecorator(DecoratorFlags.FINAL)) {
1493+
this.error(
1494+
DiagnosticCode.A_class_with_a_constructor_explicitly_returning_something_else_than_this_must_be_final,
1495+
classInstance.identifierNode.range
1496+
);
1497+
}
1498+
1499+
// Implicitly return `this` if the flow falls through
1500+
if (!flow.is(FlowFlags.TERMINATES)) {
1501+
assert(flow.isAnyLocalFlag(thisLocal.index, LocalFlags.ANY_RETAINED));
1502+
flow.unsetLocalFlag(thisLocal.index, LocalFlags.ANY_RETAINED); // undo
1503+
this.performAutoreleases(flow, stmts);
14681504
this.finishAutoreleases(flow, stmts);
14691505
stmts.push(module.local_get(thisLocal.index, this.options.nativeSizeType));
14701506
flow.set(FlowFlags.RETURNS | FlowFlags.RETURNS_NONNULL | FlowFlags.TERMINATES);
@@ -2623,6 +2659,9 @@ export class Compiler extends DiagnosticEmitter {
26232659

26242660
// take special care of properly retaining the returned value
26252661
expr = this.compileReturnedExpression(valueExpression, returnType, constraints);
2662+
if (flow.actualFunction.is(CommonFlags.CONSTRUCTOR) && valueExpression.kind != NodeKind.THIS) {
2663+
flow.set(FlowFlags.MAY_RETURN_NONTHIS);
2664+
}
26262665
} else if (returnType != Type.void) {
26272666
this.error(
26282667
DiagnosticCode.Type_0_is_not_assignable_to_type_1,
@@ -6322,44 +6361,29 @@ export class Compiler extends DiagnosticEmitter {
63226361
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
63236362
let nativeSizeType = this.options.nativeSizeType;
63246363

6325-
// {
6326-
// this = super(this || <ALLOC>, ...args)
6327-
// this.a = X
6328-
// this.b = Y
6329-
// }
6330-
let theCall = this.compileCallDirect(
6364+
let superCall = this.compileCallDirect(
63316365
this.ensureConstructor(baseClassInstance, expression),
63326366
expression.arguments,
63336367
expression,
6334-
module.if(
6335-
module.local_get(thisLocal.index, nativeSizeType),
6336-
module.local_get(thisLocal.index, nativeSizeType),
6337-
this.makeRetain(
6338-
this.makeAllocation(classInstance)
6339-
)
6340-
),
6368+
module.local_get(thisLocal.index, nativeSizeType),
63416369
Constraints.WILL_RETAIN
63426370
);
6343-
assert(baseClassInstance.type.isUnmanaged || this.skippedAutoreleases.has(theCall)); // guaranteed
6344-
let stmts: ExpressionRef[] = [
6345-
module.local_set(thisLocal.index, theCall)
6346-
];
6347-
this.makeFieldInitializationInConstructor(classInstance, stmts);
6371+
assert(baseClassInstance.type.isUnmanaged || this.skippedAutoreleases.has(superCall)); // guaranteed
63486372

63496373
// check that super had been called before accessing `this`
63506374
if (flow.isAny(
6351-
FlowFlags.ALLOCATES |
6352-
FlowFlags.CONDITIONALLY_ALLOCATES
6375+
FlowFlags.ACCESSES_THIS |
6376+
FlowFlags.CONDITIONALLY_ACCESSES_THIS
63536377
)) {
63546378
this.error(
63556379
DiagnosticCode._super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class,
63566380
expression.range
63576381
);
63586382
return module.unreachable();
63596383
}
6360-
flow.set(FlowFlags.ALLOCATES | FlowFlags.CALLS_SUPER);
6384+
flow.set(FlowFlags.ACCESSES_THIS | FlowFlags.CALLS_SUPER);
63616385
this.currentType = Type.void;
6362-
return module.flatten(stmts);
6386+
return module.local_set(thisLocal.index, superCall);
63636387
}
63646388

63656389
// otherwise resolve normally
@@ -6774,7 +6798,13 @@ export class Compiler extends DiagnosticEmitter {
67746798
let classInstance = <Class>parent;
67756799
let thisType = assert(instance.signature.thisType);
67766800
let thisLocal = flow.addScopedLocal(CommonNames.this_, thisType, usedLocals);
6777-
// No need to retain `this` as it can't be reassigned and thus can't become prematurely released
6801+
// In normal instance functions, `this` is effectively a constant
6802+
// retained elsewhere so does not need to be retained.
6803+
if (instance.is(CommonFlags.CONSTRUCTOR)) {
6804+
// Constructors, however, can allocate their own memory, and as such
6805+
// must refcount the allocation in case something else is `return`ed.
6806+
flow.setLocalFlag(thisLocal.index, LocalFlags.RETAINED);
6807+
}
67786808
body.unshift(
67796809
module.local_set(thisLocal.index, thisArg)
67806810
);
@@ -7986,41 +8016,10 @@ export class Compiler extends DiagnosticEmitter {
79868016
case NodeKind.THIS: {
79878017
if (actualFunction.is(CommonFlags.INSTANCE)) {
79888018
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
8019+
let thisType = assert(actualFunction.signature.thisType);
79898020
let parent = assert(actualFunction.parent);
79908021
assert(parent.kind == ElementKind.CLASS);
7991-
let classInstance = <Class>parent;
7992-
let nativeSizeType = this.options.nativeSizeType;
7993-
if (actualFunction.is(CommonFlags.CONSTRUCTOR)) {
7994-
if (!flow.is(FlowFlags.ALLOCATES)) {
7995-
flow.set(FlowFlags.ALLOCATES);
7996-
// {
7997-
// if (!this) this = <ALLOC>
7998-
// this.a = X
7999-
// this.b = Y
8000-
// return this
8001-
// }
8002-
let stmts: ExpressionRef[] = [
8003-
module.if(
8004-
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
8005-
module.local_get(thisLocal.index, nativeSizeType)
8006-
),
8007-
module.local_set(thisLocal.index,
8008-
this.makeRetain(
8009-
this.makeAllocation(classInstance)
8010-
)
8011-
)
8012-
)
8013-
];
8014-
this.makeFieldInitializationInConstructor(classInstance, stmts);
8015-
stmts.push(
8016-
module.local_get(thisLocal.index, nativeSizeType)
8017-
);
8018-
this.currentType = thisLocal.type;
8019-
return module.flatten(stmts, nativeSizeType);
8020-
}
8021-
}
8022-
// if not a constructor, `this` type can differ
8023-
let thisType = assert(actualFunction.signature.thisType);
8022+
flow.set(FlowFlags.ACCESSES_THIS);
80248023
this.currentType = thisType;
80258024
return module.local_get(thisLocal.index, thisType.toNativeType());
80268025
}

‎src/diagnosticMessages.generated.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export enum DiagnosticCode {
2424
Unmanaged_classes_cannot_implement_interfaces = 208,
2525
Invalid_regular_expression_flags = 209,
2626
Expression_is_never_null = 210,
27-
Class_0_is_sealed_and_cannot_be_extended = 211,
27+
Class_0_is_final_and_cannot_be_extended = 211,
2828
Decorator_0_is_not_valid_here = 212,
2929
Duplicate_decorator = 213,
3030
Type_0_is_illegal_in_this_context = 214,
@@ -44,11 +44,13 @@ export enum DiagnosticCode {
4444
Function_0_is_virtual_and_will_not_be_inlined = 228,
4545
Property_0_only_has_a_setter_and_is_missing_a_getter = 229,
4646
_0_keyword_cannot_be_used_here = 230,
47+
A_class_with_a_constructor_explicitly_returning_something_else_than_this_must_be_final = 231,
4748
Type_0_is_cyclic_Module_will_include_deferred_garbage_collection = 900,
4849
Importing_the_table_disables_some_indirect_call_optimizations = 901,
4950
Exporting_the_table_disables_some_indirect_call_optimizations = 902,
5051
Expression_compiles_to_a_dynamic_check_at_runtime = 903,
5152
Indexed_access_may_involve_bounds_checking = 904,
53+
Explicitly_returning_constructor_drops_this_allocation = 905,
5254
Unterminated_string_literal = 1002,
5355
Identifier_expected = 1003,
5456
_0_expected = 1005,
@@ -192,7 +194,7 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
192194
case 208: return "Unmanaged classes cannot implement interfaces.";
193195
case 209: return "Invalid regular expression flags.";
194196
case 210: return "Expression is never 'null'.";
195-
case 211: return "Class '{0}' is sealed and cannot be extended.";
197+
case 211: return "Class '{0}' is final and cannot be extended.";
196198
case 212: return "Decorator '{0}' is not valid here.";
197199
case 213: return "Duplicate decorator.";
198200
case 214: return "Type '{0}' is illegal in this context.";
@@ -212,11 +214,13 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
212214
case 228: return "Function '{0}' is virtual and will not be inlined.";
213215
case 229: return "Property '{0}' only has a setter and is missing a getter.";
214216
case 230: return "'{0}' keyword cannot be used here.";
217+
case 231: return "A class with a constructor explicitly returning something else than 'this' must be '@final'.";
215218
case 900: return "Type '{0}' is cyclic. Module will include deferred garbage collection.";
216219
case 901: return "Importing the table disables some indirect call optimizations.";
217220
case 902: return "Exporting the table disables some indirect call optimizations.";
218221
case 903: return "Expression compiles to a dynamic check at runtime.";
219222
case 904: return "Indexed access may involve bounds checking.";
223+
case 905: return "Explicitly returning constructor drops 'this' allocation.";
220224
case 1002: return "Unterminated string literal.";
221225
case 1003: return "Identifier expected.";
222226
case 1005: return "'{0}' expected.";

‎src/diagnosticMessages.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"Unmanaged classes cannot implement interfaces.": 208,
1818
"Invalid regular expression flags.": 209,
1919
"Expression is never 'null'.": 210,
20-
"Class '{0}' is sealed and cannot be extended.": 211,
20+
"Class '{0}' is final and cannot be extended.": 211,
2121
"Decorator '{0}' is not valid here.": 212,
2222
"Duplicate decorator.": 213,
2323
"Type '{0}' is illegal in this context.": 214,
@@ -37,12 +37,14 @@
3737
"Function '{0}' is virtual and will not be inlined.": 228,
3838
"Property '{0}' only has a setter and is missing a getter.": 229,
3939
"'{0}' keyword cannot be used here.": 230,
40+
"A class with a constructor explicitly returning something else than 'this' must be '@final'.": 231,
4041

4142
"Type '{0}' is cyclic. Module will include deferred garbage collection.": 900,
4243
"Importing the table disables some indirect call optimizations.": 901,
4344
"Exporting the table disables some indirect call optimizations.": 902,
4445
"Expression compiles to a dynamic check at runtime.": 903,
4546
"Indexed access may involve bounds checking.": 904,
47+
"Explicitly returning constructor drops 'this' allocation.": 905,
4648

4749
"Unterminated string literal.": 1002,
4850
"Identifier expected.": 1003,

0 commit comments

Comments
 (0)