Skip to content

Rework implicit allocation in constructors #1255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dcodeIO merged 7 commits into from
May 9, 2020
Merged

Rework implicit allocation in constructors #1255

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b9af1cd
Rework implicit allocation in constructors
dcodeIO May 7, 2020
1ee38ad
fix bootstrap
dcodeIO May 7, 2020
216d17c
what if
dcodeIO May 7, 2020
dbce691
fix
dcodeIO May 7, 2020
c1a91c1
allow more
dcodeIO May 8, 2020
22e69f6
rename sealed to final
dcodeIO May 8, 2020
2c0184b
minor
dcodeIO May 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 76 additions & 80 deletions src/compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,13 @@ export class Compiler extends DiagnosticEmitter {
let index = 0;
let thisType = signature.thisType;
if (thisType) {
// No need to retain `this` as it can't be reassigned and thus can't become prematurely released
// In normal instance functions, `this` is effectively a constant
// retained elsewhere so does not need to be retained.
if (instance.is(CommonFlags.CONSTRUCTOR)) {
// Constructors, however, can allocate their own memory, and as such
// must refcount the allocation in case something else is `return`ed.
flow.setLocalFlag(index, LocalFlags.RETAINED);
}
++index;
}
let parameterTypes = signature.parameterTypes;
Expand Down Expand Up @@ -1343,7 +1349,7 @@ export class Compiler extends DiagnosticEmitter {
signature.nativeParams,
signature.nativeResults,
typesToNativeTypes(instance.additionalLocals),
module.flatten(stmts, instance.signature.returnType.toNativeType())
body
);

// imported function
Expand Down Expand Up @@ -1392,6 +1398,9 @@ export class Compiler extends DiagnosticEmitter {
var bodyNode = assert(instance.prototype.bodyNode);
var returnType = instance.signature.returnType;
var flow = this.currentFlow;
var thisLocal = instance.is(CommonFlags.INSTANCE)
? assert(flow.lookupLocal(CommonNames.this_))
: null;

// compile statements
if (bodyNode.kind == NodeKind.BLOCK) {
Expand Down Expand Up @@ -1432,39 +1441,66 @@ export class Compiler extends DiagnosticEmitter {
}
}

// make constructors return their instance pointer
// Make constructors return their instance pointer, and prepend a conditional
// allocation if any code path accesses `this`.
if (instance.is(CommonFlags.CONSTRUCTOR)) {
let nativeSizeType = this.options.nativeSizeType;
assert(instance.is(CommonFlags.INSTANCE));
thisLocal = assert(thisLocal);
let parent = assert(instance.parent);
assert(parent.kind == ElementKind.CLASS);
let classInstance = <Class>parent;

if (!flow.is(FlowFlags.TERMINATES)) {
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));

// if `this` wasn't accessed before, allocate if necessary and initialize `this`
if (!flow.is(FlowFlags.ALLOCATES)) {
// {
// if (!this) this = <ALLOC>
// this.a = X
// this.b = Y
// }
stmts.push(
module.if(
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
module.local_get(thisLocal.index, nativeSizeType)
),
module.local_set(thisLocal.index,
this.makeRetain(
this.makeAllocation(classInstance)
),
if (flow.isAny(FlowFlags.ACCESSES_THIS | FlowFlags.CONDITIONALLY_ACCESSES_THIS) || !flow.is(FlowFlags.TERMINATES)) {
// Allocate `this` if not a super call, and initialize fields
let allocStmts = new Array<ExpressionRef>();
allocStmts.push(
module.if(
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
module.local_get(thisLocal.index, nativeSizeType)
),
module.local_set(thisLocal.index,
this.makeRetain(
this.makeAllocation(classInstance)
)
)
)
);
this.makeFieldInitializationInConstructor(classInstance, allocStmts);
if (flow.isInline) {
let firstStmt = stmts[0]; // `this` alias assignment
assert(getExpressionId(firstStmt) == ExpressionId.LocalSet);
assert(getLocalSetIndex(firstStmt) == thisLocal.index);
allocStmts.unshift(firstStmt);
stmts[0] = module.flatten(allocStmts, NativeType.None);
} else {
stmts.unshift(
module.flatten(allocStmts, NativeType.None)
);
}
}

// Check explicit return conditions if applicable
if (flow.isAny(FlowFlags.RETURNS | FlowFlags.CONDITIONALLY_RETURNS)) {
if (flow.isAny(FlowFlags.ACCESSES_THIS | FlowFlags.CONDITIONALLY_ACCESSES_THIS)) {
this.error(
DiagnosticCode.An_explicitly_returning_constructor_must_not_access_this,
instance.identifierNode.range
);
}
if (!classInstance.hasDecorator(DecoratorFlags.SEALED)) {
this.error(
DiagnosticCode.A_class_with_an_explicitly_returning_constructor_must_be_sealed,
classInstance.identifierNode.range
);
this.makeFieldInitializationInConstructor(classInstance, stmts);
}
this.performAutoreleases(flow, stmts); // `this` is excluded anyway
}

// Implicitly return `this` if the flow falls through
if (!flow.is(FlowFlags.TERMINATES)) {
assert(flow.isAnyLocalFlag(thisLocal.index, LocalFlags.ANY_RETAINED));
flow.unsetLocalFlag(thisLocal.index, LocalFlags.ANY_RETAINED); // undo
this.performAutoreleases(flow, stmts);
this.finishAutoreleases(flow, stmts);
stmts.push(module.local_get(thisLocal.index, this.options.nativeSizeType));
flow.set(FlowFlags.RETURNS | FlowFlags.RETURNS_NONNULL | FlowFlags.TERMINATES);
Expand Down Expand Up @@ -6322,44 +6358,29 @@ export class Compiler extends DiagnosticEmitter {
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
let nativeSizeType = this.options.nativeSizeType;

// {
// this = super(this || <ALLOC>, ...args)
// this.a = X
// this.b = Y
// }
let theCall = this.compileCallDirect(
let superCall = this.compileCallDirect(
this.ensureConstructor(baseClassInstance, expression),
expression.arguments,
expression,
module.if(
module.local_get(thisLocal.index, nativeSizeType),
module.local_get(thisLocal.index, nativeSizeType),
this.makeRetain(
this.makeAllocation(classInstance)
)
),
module.local_get(thisLocal.index, nativeSizeType),
Constraints.WILL_RETAIN
);
assert(baseClassInstance.type.isUnmanaged || this.skippedAutoreleases.has(theCall)); // guaranteed
let stmts: ExpressionRef[] = [
module.local_set(thisLocal.index, theCall)
];
this.makeFieldInitializationInConstructor(classInstance, stmts);
assert(baseClassInstance.type.isUnmanaged || this.skippedAutoreleases.has(superCall)); // guaranteed

// check that super had been called before accessing `this`
if (flow.isAny(
FlowFlags.ALLOCATES |
FlowFlags.CONDITIONALLY_ALLOCATES
FlowFlags.ACCESSES_THIS |
FlowFlags.CONDITIONALLY_ACCESSES_THIS
)) {
this.error(
DiagnosticCode._super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class,
expression.range
);
return module.unreachable();
}
flow.set(FlowFlags.ALLOCATES | FlowFlags.CALLS_SUPER);
flow.set(FlowFlags.ACCESSES_THIS | FlowFlags.CALLS_SUPER);
this.currentType = Type.void;
return module.flatten(stmts);
return module.local_set(thisLocal.index, superCall);
}

// otherwise resolve normally
Expand Down Expand Up @@ -6774,7 +6795,13 @@ export class Compiler extends DiagnosticEmitter {
let classInstance = <Class>parent;
let thisType = assert(instance.signature.thisType);
let thisLocal = flow.addScopedLocal(CommonNames.this_, thisType, usedLocals);
// No need to retain `this` as it can't be reassigned and thus can't become prematurely released
// In normal instance functions, `this` is effectively a constant
// retained elsewhere so does not need to be retained.
if (instance.is(CommonFlags.CONSTRUCTOR)) {
// Constructors, however, can allocate their own memory, and as such
// must refcount the allocation in case something else is `return`ed.
flow.setLocalFlag(thisLocal.index, LocalFlags.RETAINED);
}
body.unshift(
module.local_set(thisLocal.index, thisArg)
);
Expand Down Expand Up @@ -7986,41 +8013,10 @@ export class Compiler extends DiagnosticEmitter {
case NodeKind.THIS: {
if (actualFunction.is(CommonFlags.INSTANCE)) {
let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
let thisType = assert(actualFunction.signature.thisType);
let parent = assert(actualFunction.parent);
assert(parent.kind == ElementKind.CLASS);
let classInstance = <Class>parent;
let nativeSizeType = this.options.nativeSizeType;
if (actualFunction.is(CommonFlags.CONSTRUCTOR)) {
if (!flow.is(FlowFlags.ALLOCATES)) {
flow.set(FlowFlags.ALLOCATES);
// {
// if (!this) this = <ALLOC>
// this.a = X
// this.b = Y
// return this
// }
let stmts: ExpressionRef[] = [
module.if(
module.unary(nativeSizeType == NativeType.I64 ? UnaryOp.EqzI64 : UnaryOp.EqzI32,
module.local_get(thisLocal.index, nativeSizeType)
),
module.local_set(thisLocal.index,
this.makeRetain(
this.makeAllocation(classInstance)
)
)
)
];
this.makeFieldInitializationInConstructor(classInstance, stmts);
stmts.push(
module.local_get(thisLocal.index, nativeSizeType)
);
this.currentType = thisLocal.type;
return module.flatten(stmts, nativeSizeType);
}
}
// if not a constructor, `this` type can differ
let thisType = assert(actualFunction.signature.thisType);
flow.set(FlowFlags.ACCESSES_THIS);
this.currentType = thisType;
return module.local_get(thisLocal.index, thisType.toNativeType());
}
Expand Down
4 changes: 4 additions & 0 deletions src/diagnosticMessages.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export enum DiagnosticCode {
Function_0_is_virtual_and_will_not_be_inlined = 228,
Property_0_only_has_a_setter_and_is_missing_a_getter = 229,
_0_keyword_cannot_be_used_here = 230,
An_explicitly_returning_constructor_must_not_access_this = 231,
A_class_with_an_explicitly_returning_constructor_must_be_sealed = 232,
Type_0_is_cyclic_Module_will_include_deferred_garbage_collection = 900,
Importing_the_table_disables_some_indirect_call_optimizations = 901,
Exporting_the_table_disables_some_indirect_call_optimizations = 902,
Expand Down Expand Up @@ -212,6 +214,8 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
case 228: return "Function '{0}' is virtual and will not be inlined.";
case 229: return "Property '{0}' only has a setter and is missing a getter.";
case 230: return "'{0}' keyword cannot be used here.";
case 231: return "An explicitly returning constructor must not access 'this'.";
case 232: return "A class with an explicitly returning constructor must be sealed.";
case 900: return "Type '{0}' is cyclic. Module will include deferred garbage collection.";
case 901: return "Importing the table disables some indirect call optimizations.";
case 902: return "Exporting the table disables some indirect call optimizations.";
Expand Down
2 changes: 2 additions & 0 deletions src/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
"Function '{0}' is virtual and will not be inlined.": 228,
"Property '{0}' only has a setter and is missing a getter.": 229,
"'{0}' keyword cannot be used here.": 230,
"An explicitly returning constructor must not access 'this'.": 231,
"A class with an explicitly returning constructor must be sealed.": 232,

"Type '{0}' is cyclic. Module will include deferred garbage collection.": 900,
"Importing the table disables some indirect call optimizations.": 901,
Expand Down
46 changes: 23 additions & 23 deletions src/flow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ export const enum FlowFlags {
BREAKS = 1 << 4,
/** This flow always continues. */
CONTINUES = 1 << 5,
/** This flow always allocates. Constructors only. */
ALLOCATES = 1 << 6,
/** This flow always calls super. Constructors only. */
/** This flow always accesses `this`. Constructors only. */
ACCESSES_THIS = 1 << 6,
/** This flow always calls `super`. Constructors only. */
CALLS_SUPER = 1 << 7,
/** This flow always terminates (returns, throws or continues). */
TERMINATES = 1 << 8, // Note that this doesn't cover BREAKS, which is separate
Expand All @@ -116,13 +116,13 @@ export const enum FlowFlags {
CONDITIONALLY_BREAKS = 1 << 11,
/** This flow conditionally continues in a child flow. */
CONDITIONALLY_CONTINUES = 1 << 12,
/** This flow conditionally allocates in a child flow. Constructors only. */
CONDITIONALLY_ALLOCATES = 1 << 13,
/** This flow conditionally accesses `this` in a child flow. Constructors only. */
CONDITIONALLY_ACCESSES_THIS = 1 << 13,

// other

/** This is a flow with explicitly disabled bounds checking. */
UNCHECKED_CONTEXT = 1 << 15,
UNCHECKED_CONTEXT = 1 << 14,

// masks

Expand All @@ -133,7 +133,7 @@ export const enum FlowFlags {
| FlowFlags.THROWS
| FlowFlags.BREAKS
| FlowFlags.CONTINUES
| FlowFlags.ALLOCATES
| FlowFlags.ACCESSES_THIS
| FlowFlags.CALLS_SUPER
| FlowFlags.TERMINATES,

Expand All @@ -142,7 +142,7 @@ export const enum FlowFlags {
| FlowFlags.CONDITIONALLY_THROWS
| FlowFlags.CONDITIONALLY_BREAKS
| FlowFlags.CONDITIONALLY_CONTINUES
| FlowFlags.CONDITIONALLY_ALLOCATES
| FlowFlags.CONDITIONALLY_ACCESSES_THIS
}

/** Flags indicating the current state of a local. */
Expand Down Expand Up @@ -627,14 +627,14 @@ export class Flow {
newFlags |= thisFlags & FlowFlags.CONDITIONALLY_CONTINUES;
}

if (thisFlags & FlowFlags.ALLOCATES) { // can become conditional
if (otherFlags & FlowFlags.ALLOCATES) {
newFlags |= FlowFlags.ALLOCATES;
if (thisFlags & FlowFlags.ACCESSES_THIS) { // can become conditional
if (otherFlags & FlowFlags.ACCESSES_THIS) {
newFlags |= FlowFlags.ACCESSES_THIS;
} else {
newFlags |= FlowFlags.CONDITIONALLY_ALLOCATES;
newFlags |= FlowFlags.CONDITIONALLY_ACCESSES_THIS;
}
} else if (otherFlags & FlowFlags.ALLOCATES) {
newFlags |= FlowFlags.CONDITIONALLY_ALLOCATES;
} else if (otherFlags & FlowFlags.ACCESSES_THIS) {
newFlags |= FlowFlags.CONDITIONALLY_ACCESSES_THIS;
}

// must be the case in both
Expand Down Expand Up @@ -742,16 +742,16 @@ export class Flow {
newFlags |= (leftFlags | rightFlags) & FlowFlags.CONDITIONALLY_CONTINUES;
}

if (leftFlags & FlowFlags.ALLOCATES) {
if (rightFlags & FlowFlags.ALLOCATES) {
newFlags |= FlowFlags.ALLOCATES;
if (leftFlags & FlowFlags.ACCESSES_THIS) {
if (rightFlags & FlowFlags.ACCESSES_THIS) {
newFlags |= FlowFlags.ACCESSES_THIS;
} else {
newFlags |= FlowFlags.CONDITIONALLY_ALLOCATES;
newFlags |= FlowFlags.CONDITIONALLY_ACCESSES_THIS;
}
} else if (rightFlags & FlowFlags.ALLOCATES) {
newFlags |= FlowFlags.CONDITIONALLY_ALLOCATES;
} else if (rightFlags & FlowFlags.ACCESSES_THIS) {
newFlags |= FlowFlags.CONDITIONALLY_ACCESSES_THIS;
} else {
newFlags |= (leftFlags | rightFlags) & FlowFlags.CONDITIONALLY_ALLOCATES;
newFlags |= (leftFlags | rightFlags) & FlowFlags.CONDITIONALLY_ACCESSES_THIS;
}

if ((leftFlags & FlowFlags.CALLS_SUPER) && (rightFlags & FlowFlags.CALLS_SUPER)) {
Expand Down Expand Up @@ -1344,14 +1344,14 @@ export class Flow {
if (this.is(FlowFlags.THROWS)) sb.push("THROWS");
if (this.is(FlowFlags.BREAKS)) sb.push("BREAKS");
if (this.is(FlowFlags.CONTINUES)) sb.push("CONTINUES");
if (this.is(FlowFlags.ALLOCATES)) sb.push("ALLOCATES");
if (this.is(FlowFlags.ACCESSES_THIS)) sb.push("ACCESS_THIS");
if (this.is(FlowFlags.CALLS_SUPER)) sb.push("CALLS_SUPER");
if (this.is(FlowFlags.TERMINATES)) sb.push("TERMINATES");
if (this.is(FlowFlags.CONDITIONALLY_RETURNS)) sb.push("CONDITIONALLY_RETURNS");
if (this.is(FlowFlags.CONDITIONALLY_THROWS)) sb.push("CONDITIONALLY_THROWS");
if (this.is(FlowFlags.CONDITIONALLY_BREAKS)) sb.push("CONDITIONALLY_BREAKS");
if (this.is(FlowFlags.CONDITIONALLY_CONTINUES)) sb.push("CONDITIONALLY_CONTINUES");
if (this.is(FlowFlags.CONDITIONALLY_ALLOCATES)) sb.push("CONDITIONALLY_ALLOCATES");
if (this.is(FlowFlags.CONDITIONALLY_ACCESSES_THIS)) sb.push("CONDITIONALLY_ACCESS_THIS");
return "Flow(" + this.actualFunction.toString() + ")[" + levels.toString() + "] " + sb.join(" ");
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,15 +577,19 @@ export class Signature {

var signatureTypes = program.uniqueSignatures;
var length = signatureTypes.length;
var isUnique = true;
for (let i = 0; i < length; i++) {
let compare = signatureTypes[i];
if (this.equals(compare)) {
this.id = compare.id;
return this;
isUnique = false;
break;
}
}
program.uniqueSignatures.push(this);
this.id = program.nextSignatureId++;
if (isUnique) {
this.id = program.nextSignatureId++;
program.uniqueSignatures.push(this);
}
}

get nativeParams(): NativeType {
Expand Down
Loading