Skip to content

Fix missing reference conversion case #1058

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.

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
merged 8 commits into from
Jan 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 1 addition & 44 deletions src/builtins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2170,7 +2170,7 @@ export function compileCall(
}

// otherwise call abort if the assertion is false-ish
let abort = compileAbort(compiler, operands.length == 2 ? operands[1] : null, reportNode);
let abort = compiler.makeAbort(operands.length == 2 ? operands[1] : null, reportNode);
compiler.currentType = type.nonNullableType;
if (contextualType == Type.void) { // simplify if dropped anyway
compiler.currentType = Type.void;
Expand Down Expand Up @@ -4726,49 +4726,6 @@ function deferASM(
);
}

/** Compiles an abort wired to the conditionally imported 'abort' function. */
export function compileAbort(
compiler: Compiler,
message: Expression | null,
reportNode: Node
): ExpressionRef {
var program = compiler.program;
var module = compiler.module;

var stringInstance = compiler.program.stringInstance;
if (!stringInstance) return module.unreachable();

var abortInstance = program.abortInstance;
if (!(abortInstance && compiler.compileFunction(abortInstance))) return module.unreachable();

var messageArg: ExpressionRef;
if (message !== null) {
// The message argument works much like an arm of an IF that does not become executed if the
// assertion succeeds respectively is only being computed if the program actually crashes.
// Hence, let's make it so that the autorelease is skipped at the end of the current block,
// essentially ignoring the message GC-wise. Doesn't matter anyway on a crash.
messageArg = compiler.compileExpression(message, stringInstance.type, Constraints.CONV_IMPLICIT | Constraints.WILL_RETAIN);
} else {
messageArg = compiler.makeZero(stringInstance.type);
}

var filenameArg = compiler.ensureStaticString(reportNode.range.source.normalizedPath);

compiler.currentType = Type.void;
return module.block(null, [
module.call(
abortInstance.internalName, [
messageArg,
filenameArg,
module.i32(reportNode.range.line),
module.i32(reportNode.range.column)
],
NativeType.None
),
module.unreachable()
]);
}

/** Compiles the `visit_globals` function. */
export function compileVisitGlobals(compiler: Compiler): void {
var module = compiler.module;
Expand Down
162 changes: 136 additions & 26 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import {
BuiltinNames,
compileCall as compileBuiltinCall,
compileAbort,
compileVisitGlobals,
compileVisitMembers,
compileRTTI,
Expand Down Expand Up @@ -2499,7 +2498,9 @@ export class Compiler extends DiagnosticEmitter {
let newArgs = (<NewExpression>value).arguments;
if (newArgs.length) message = newArgs[0]; // FIXME: naively assumes type string
}
stmts.push(compileAbort(this, message, statement));
stmts.push(
this.makeAbort(message, statement)
);

return this.module.flatten(stmts);
}
Expand Down Expand Up @@ -3125,21 +3126,48 @@ export class Compiler extends DiagnosticEmitter {
// any to void
if (toType.kind == TypeKind.VOID) return module.drop(expr);

if (this.currentFlow.isNonnull(expr, fromType)) fromType = fromType.nonNullableType;
// reference involved
if (fromType.is(TypeFlags.REFERENCE) || toType.is(TypeFlags.REFERENCE)) {
if (this.currentFlow.isNonnull(expr, fromType)) {
fromType = fromType.nonNullableType;
} else if (explicit && fromType.is(TypeFlags.NULLABLE) && !toType.is(TypeFlags.NULLABLE)) {
// explicit conversion from nullable to non-nullable requires a runtime
// check here because nonnull state above already didn't know better
if (!this.options.noAssert) {
expr = this.makeRuntimeNonNullCheck(expr, fromType, reportNode);
}
fromType = fromType.nonNullableType;
}
if (fromType.isAssignableTo(toType)) { // downcast or same
assert(fromType.kind == toType.kind);
this.currentType = toType;
return expr;
}
if (explicit && toType.nonNullableType.isAssignableTo(fromType)) { // upcast
// <Cat | null>(<Animal>maybeCat)
assert(fromType.kind == toType.kind);
if (!this.options.noAssert) {
expr = this.makeRuntimeUpcastCheck(expr, fromType, toType, reportNode);
}
this.currentType = toType;
return expr;
}
this.error(
DiagnosticCode.Type_0_is_not_assignable_to_type_1,
reportNode.range, fromType.toString(), toType.toString()
);
this.currentType = toType;
return module.unreachable();
}

// not dealing with references from here on

if (!fromType.isAssignableTo(toType)) {
if (!explicit) {
if (fromType.nonNullableType == toType) {
this.error(
DiagnosticCode.Object_is_possibly_null,
reportNode.range
); // recoverable
} else {
this.error(
DiagnosticCode.Conversion_from_type_0_to_1_requires_an_explicit_cast,
reportNode.range, fromType.toString(), toType.toString()
); // recoverable
}
this.error(
DiagnosticCode.Conversion_from_type_0_to_1_requires_an_explicit_cast,
reportNode.range, fromType.toString(), toType.toString()
); // recoverable
}
}

Expand Down Expand Up @@ -3319,19 +3347,9 @@ export class Compiler extends DiagnosticEmitter {
expression.expression.range
);
} else if (!this.options.noAssert) {
let module = this.module;
let flow = this.currentFlow;
let temp = flow.getTempLocal(type);
if (!flow.canOverflow(expr, type)) flow.setLocalFlag(temp.index, LocalFlags.WRAPPED);
flow.setLocalFlag(temp.index, LocalFlags.NONNULL);
expr = module.if(
module.local_tee(temp.index, expr),
module.local_get(temp.index, type.toNativeType()),
module.unreachable()
);
flow.freeTempLocal(temp);
expr = this.makeRuntimeNonNullCheck(expr, type, expression);
}
this.currentType = this.currentType.nonNullableType;
this.currentType = type.nonNullableType;
return expr;
}
default: assert(false);
Expand Down Expand Up @@ -9376,6 +9394,98 @@ export class Compiler extends DiagnosticEmitter {
}
return stmts;
}

/** Makes a call to `abort`, if present, otherwise creates a trap. */
makeAbort(
/** Message argument of type string, if any. */
message: Expression | null,
/** Code location to report when aborting. */
codeLocation: Node
): ExpressionRef {
var program = this.program;
var module = this.module;
var stringInstance = program.stringInstance;
var abortInstance = program.abortInstance;
if (!abortInstance || !this.compileFunction(abortInstance)) return module.unreachable();

var messageArg: ExpressionRef;
if (message !== null) {
// The message argument works much like an arm of an IF that does not become executed if the
// assertion succeeds respectively is only being computed if the program actually crashes.
// Hence, let's make it so that the autorelease is skipped at the end of the current block,
// essentially ignoring the message GC-wise. Doesn't matter anyway on a crash.
messageArg = this.compileExpression(message, stringInstance.type, Constraints.CONV_IMPLICIT | Constraints.WILL_RETAIN);
} else {
messageArg = this.makeZero(stringInstance.type);
}

var filenameArg = this.ensureStaticString(codeLocation.range.source.normalizedPath);
return module.block(null, [
module.call(
abortInstance.internalName, [
messageArg,
filenameArg,
module.i32(codeLocation.range.line),
module.i32(codeLocation.range.column)
],
NativeType.None
),
module.unreachable()
]);
}

/** Makes a runtime non-null check, e.g. on `<Type>possiblyNull` or `possiblyNull!`. */
makeRuntimeNonNullCheck(
/** Expression being checked. */
expr: ExpressionRef,
/** Type of the expression. */
type: Type,
/** Report node. */
reportNode: Node
): ExpressionRef {
assert(type.is(TypeFlags.NULLABLE | TypeFlags.REFERENCE));
var module = this.module;
var flow = this.currentFlow;
var temp = flow.getTempLocal(type);
if (!flow.canOverflow(expr, type)) flow.setLocalFlag(temp.index, LocalFlags.WRAPPED);
flow.setLocalFlag(temp.index, LocalFlags.NONNULL);
expr = module.if(
module.local_tee(temp.index, expr),
module.local_get(temp.index, type.toNativeType()),
this.makeAbort(null, reportNode) // TODO: throw
);
flow.freeTempLocal(temp);
return expr;
}

/** Makes a runtime upcast check, e.g. on `<Child>parent`. */
makeRuntimeUpcastCheck(
/** Expression being upcast. */
expr: ExpressionRef,
/** Type of the expression. */
type: Type,
/** Type casting to. */
toType: Type,
/** Report node. */
reportNode: Node
): ExpressionRef {
assert(toType.is(TypeFlags.REFERENCE) && toType.nonNullableType.isAssignableTo(type));
var module = this.module;
var flow = this.currentFlow;
var temp = flow.getTempLocal(type);
var instanceofInstance = this.program.instanceofInstance;
assert(this.compileFunction(instanceofInstance));
expr = module.if(
module.call(instanceofInstance.internalName, [
module.local_tee(temp.index, expr),
module.i32(assert(toType.classReference).id)
], NativeType.I32),
module.local_get(temp.index, type.toNativeType()),
this.makeAbort(null, reportNode) // TODO: throw
);
flow.freeTempLocal(temp);
return expr;
}
}

// helpers
Expand Down
4 changes: 2 additions & 2 deletions std/assembly/rt/tlsf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,12 +498,12 @@ export function allocateBlock(root: Root, size: usize): Block {
block = searchBlock(root, payloadSize);
if (!block) {
growMemory(root, payloadSize);
block = <Block>searchBlock(root, payloadSize);
block = changetype<Block>(searchBlock(root, payloadSize));
if (DEBUG) assert(block); // must be found now
}
} else {
growMemory(root, payloadSize);
block = <Block>searchBlock(root, payloadSize);
block = changetype<Block>(searchBlock(root, payloadSize));
if (DEBUG) assert(block); // must be found now
}
}
Expand Down
Loading