Skip to content
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

Jump to bottom

Rework implicit allocation in constructors #1255

Merged
merged 7 commits into from
May 9, 2020
Merged

Rework implicit allocation in constructors #1255

Show file tree
Hide file tree
Changes from all 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
12 changes: 5 additions & 7 deletions src/ast.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,7 @@ export enum DecoratorKind {
OPERATOR_PREFIX,
OPERATOR_POSTFIX,
UNMANAGED,
SEALED,
FINAL,
INLINE,
EXTERNAL,
BUILTIN,
Expand All @@ -1316,7 +1316,6 @@ export namespace DecoratorKind {

/** Returns the kind of the specified decorator name node. Defaults to {@link DecoratorKind.CUSTOM}. */
export function fromNode(nameNode: Expression): DecoratorKind {
// @global, @inline, @operator, @sealed, @unmanaged
if (nameNode.kind == NodeKind.IDENTIFIER) {
let nameStr = (<IdentifierExpression>nameNode).text;
assert(nameStr.length);
Expand All @@ -1329,6 +1328,10 @@ export namespace DecoratorKind {
if (nameStr == "external") return DecoratorKind.EXTERNAL;
break;
}
case CharCode.f: {
if (nameStr == "final") return DecoratorKind.FINAL;
break;
}
case CharCode.g: {
if (nameStr == "global") return DecoratorKind.GLOBAL;
break;
Expand All @@ -1345,10 +1348,6 @@ export namespace DecoratorKind {
if (nameStr == "operator") return DecoratorKind.OPERATOR;
break;
}
case CharCode.s: {
if (nameStr == "sealed") return DecoratorKind.SEALED;
break;
}
case CharCode.u: {
if (nameStr == "unmanaged") return DecoratorKind.UNMANAGED;
if (nameStr == "unsafe") return DecoratorKind.UNSAFE;
Expand All @@ -1363,7 +1362,6 @@ export namespace DecoratorKind {
assert(nameStr.length);
let propStr = propertyAccessNode.property.text;
assert(propStr.length);
// @operator.binary, @operator.prefix, @operator.postfix
if (nameStr == "operator") {
switch (propStr.charCodeAt(0)) {
case CharCode.b: {
Expand Down
159 changes: 79 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)
);
this.makeFieldInitializationInConstructor(classInstance, stmts);
}
this.performAutoreleases(flow, stmts); // `this` is excluded anyway

// Just prepended allocation is dropped when returning non-'this'
if (flow.is(FlowFlags.MAY_RETURN_NONTHIS)) {
this.pedantic(
DiagnosticCode.Explicitly_returning_constructor_drops_this_allocation,
instance.identifierNode.range
);
}
}

// Returning something else than 'this' would break 'super()' calls
if (flow.is(FlowFlags.MAY_RETURN_NONTHIS) && !classInstance.hasDecorator(DecoratorFlags.FINAL)) {
this.error(
DiagnosticCode.A_class_with_a_constructor_explicitly_returning_something_else_than_this_must_be_final,
classInstance.identifierNode.range
);
}

// 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 @@ -2623,6 +2659,9 @@ export class Compiler extends DiagnosticEmitter {

// take special care of properly retaining the returned value
expr = this.compileReturnedExpression(valueExpression, returnType, constraints);
if (flow.actualFunction.is(CommonFlags.CONSTRUCTOR) && valueExpression.kind != NodeKind.THIS) {
flow.set(FlowFlags.MAY_RETURN_NONTHIS);
}
} else if (returnType != Type.void) {
this.error(
DiagnosticCode.Type_0_is_not_assignable_to_type_1,
Expand Down Expand Up @@ -6322,44 +6361,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 +6798,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 +8016,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
8 changes: 6 additions & 2 deletions src/diagnosticMessages.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export enum DiagnosticCode {
Unmanaged_classes_cannot_implement_interfaces = 208,
Invalid_regular_expression_flags = 209,
Expression_is_never_null = 210,
Class_0_is_sealed_and_cannot_be_extended = 211,
Class_0_is_final_and_cannot_be_extended = 211,
Decorator_0_is_not_valid_here = 212,
Duplicate_decorator = 213,
Type_0_is_illegal_in_this_context = 214,
Expand All @@ -44,11 +44,13 @@ 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,
A_class_with_a_constructor_explicitly_returning_something_else_than_this_must_be_final = 231,
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,
Expression_compiles_to_a_dynamic_check_at_runtime = 903,
Indexed_access_may_involve_bounds_checking = 904,
Explicitly_returning_constructor_drops_this_allocation = 905,
Unterminated_string_literal = 1002,
Identifier_expected = 1003,
_0_expected = 1005,
Expand Down Expand Up @@ -192,7 +194,7 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
case 208: return "Unmanaged classes cannot implement interfaces.";
case 209: return "Invalid regular expression flags.";
case 210: return "Expression is never 'null'.";
case 211: return "Class '{0}' is sealed and cannot be extended.";
case 211: return "Class '{0}' is final and cannot be extended.";
case 212: return "Decorator '{0}' is not valid here.";
case 213: return "Duplicate decorator.";
case 214: return "Type '{0}' is illegal in this context.";
Expand All @@ -212,11 +214,13 @@ 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 "A class with a constructor explicitly returning something else than 'this' must be '@final'.";
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.";
case 903: return "Expression compiles to a dynamic check at runtime.";
case 904: return "Indexed access may involve bounds checking.";
case 905: return "Explicitly returning constructor drops 'this' allocation.";
case 1002: return "Unterminated string literal.";
case 1003: return "Identifier expected.";
case 1005: return "'{0}' expected.";
Expand Down
4 changes: 3 additions & 1 deletion src/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"Unmanaged classes cannot implement interfaces.": 208,
"Invalid regular expression flags.": 209,
"Expression is never 'null'.": 210,
"Class '{0}' is sealed and cannot be extended.": 211,
"Class '{0}' is final and cannot be extended.": 211,
"Decorator '{0}' is not valid here.": 212,
"Duplicate decorator.": 213,
"Type '{0}' is illegal in this context.": 214,
Expand All @@ -37,12 +37,14 @@
"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,
"A class with a constructor explicitly returning something else than 'this' must be '@final'.": 231,

"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,
"Expression compiles to a dynamic check at runtime.": 903,
"Indexed access may involve bounds checking.": 904,
"Explicitly returning constructor drops 'this' allocation.": 905,

"Unterminated string literal.": 1002,
"Identifier expected.": 1003,
Expand Down
Loading