Report if a function isn't guaranteed to return

This commit is contained in:
dcodeIO 2018-01-19 04:16:18 +01:00
parent 7be4f9fabb
commit ef7a095494
9 changed files with 131 additions and 30 deletions

View File

@ -749,4 +749,4 @@ function check_pool(pool: usize): i32 {
return integrity_status;
}
// export { check, check_pool }; // Uncomment to enable in tests/index.js
export { check, check_pool }; // Uncomment to enable in tests/index.js

View File

@ -203,7 +203,7 @@ export class Compiler extends DiagnosticEmitter {
var startFunctionTemplate = new FunctionPrototype(program, "start", "start", null);
var startFunctionInstance = new Function(startFunctionTemplate, startFunctionTemplate.internalName, [], [], Type.void, null);
this.currentFunction = this.startFunction = startFunctionInstance;
}
}
/** Performs compilation of the underlying {@link Program} to a {@link Module}. */
compile(): Module {
@ -568,7 +568,12 @@ export class Compiler extends DiagnosticEmitter {
if (!instance.is(ElementFlags.DECLARED)) {
var previousFunction = this.currentFunction;
this.currentFunction = instance;
stmts = this.compileStatements(<Statement[]>declaration.statements);
var statements = assert(declaration.statements);
stmts = this.compileStatements(statements);
// make sure the top-level branch or all child branches return
var allBranchesReturn = this.currentFunction.flow.finalize();
if (instance.returnType != Type.void && !allBranchesReturn)
this.error(DiagnosticCode.A_function_whose_declared_type_is_not_void_must_return_a_value, assert(declaration.returnType).range);
this.currentFunction = previousFunction;
}
@ -854,12 +859,15 @@ export class Compiler extends DiagnosticEmitter {
// optimizer.
// Not actually a branch, but can contain its own scoped variables.
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
var stmt = this.module.createBlock(null, this.compileStatements(statements), NativeType.None);
var stmtReturns = this.currentFunction.flow.is(FlowFlags.RETURNS);
// Switch back to the parent flow
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
if (stmtReturns)
this.currentFunction.flow.set(FlowFlags.RETURNS);
return stmt;
}
@ -938,7 +946,7 @@ export class Compiler extends DiagnosticEmitter {
// A for statement initiates a new branch with its own scoped variables
// possibly declared in its initializer, and break context.
var context = this.currentFunction.enterBreakContext();
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
var breakLabel = this.currentFunction.flow.breakLabel = "break|" + context;
var continueLabel = this.currentFunction.flow.continueLabel = "continue|" + context;
@ -947,12 +955,14 @@ export class Compiler extends DiagnosticEmitter {
var condition = statement.condition ? this.compileExpression(<Expression>statement.condition, Type.i32) : this.module.createI32(1);
var incrementor = statement.incrementor ? this.compileExpression(<Expression>statement.incrementor, Type.void) : this.module.createNop();
var body = this.compileStatement(statement.statement);
var alwaysReturns = !statement.condition && this.currentFunction.flow.is(FlowFlags.RETURNS);
// TODO: check other always-true conditions as well, not just omitted
// Switch back to the parent flow
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
this.currentFunction.leaveBreakContext();
return this.module.createBlock(breakLabel, [
var expr = this.module.createBlock(breakLabel, [
initializer,
this.module.createLoop(continueLabel, this.module.createBlock(null, [
this.module.createIf(condition, this.module.createBlock(null, [
@ -962,6 +972,16 @@ export class Compiler extends DiagnosticEmitter {
], NativeType.None))
], NativeType.None))
], NativeType.None);
// If the loop is guaranteed to run and return, propagate that and append a hint
if (alwaysReturns) {
this.currentFunction.flow.set(FlowFlags.RETURNS);
expr = this.module.createBlock(null, [
expr,
this.module.createUnreachable()
]);
}
return expr;
}
compileIfStatement(statement: IfStatement): ExpressionRef {
@ -970,16 +990,21 @@ export class Compiler extends DiagnosticEmitter {
var condition = this.compileExpression(statement.condition, Type.i32);
// Each arm initiates a branch
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
var ifTrue = this.compileStatement(statement.ifTrue);
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
var ifTrueReturns = this.currentFunction.flow.is(FlowFlags.RETURNS);
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
var ifFalse: ExpressionRef = 0;
var ifFalseReturns = false;
if (statement.ifFalse) {
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
ifFalse = this.compileStatement(statement.ifFalse);
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
ifFalseReturns = this.currentFunction.flow.is(FlowFlags.RETURNS);
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
}
if (ifTrueReturns && ifFalseReturns) // not necessary to append a hint
this.currentFunction.flow.set(FlowFlags.RETURNS);
return this.module.createIf(condition, ifTrue, ifFalse);
}
@ -1035,6 +1060,7 @@ export class Compiler extends DiagnosticEmitter {
// nest blocks in order
var currentBlock = this.module.createBlock("case0|" + context, breaks, NativeType.None);
var alwaysReturns = true;
for (i = 0; i < k; ++i) {
case_ = statement.cases[i];
var l = case_.statements.length;
@ -1042,26 +1068,38 @@ export class Compiler extends DiagnosticEmitter {
body[0] = currentBlock;
// Each switch case initiates a new branch
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
var breakLabel = this.currentFunction.flow.breakLabel = "break|" + context;
var nextLabel = i == k - 1 ? breakLabel : "case" + (i + 1).toString(10) + "|" + context;
var fallsThrough = i != k - 1;
var nextLabel = !fallsThrough ? breakLabel : "case" + (i + 1).toString(10) + "|" + context;
for (var j = 0; j < l; ++j)
body[j + 1] = this.compileStatement(case_.statements[j]);
if (!(fallsThrough || this.currentFunction.flow.is(FlowFlags.RETURNS)))
alwaysReturns = false; // ignore fall-throughs
// Switch back to the parent flow
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
currentBlock = this.module.createBlock(nextLabel, body, NativeType.None);
}
this.currentFunction.leaveBreakContext();
// If the switch has a default and always returns, propagate that
if (defaultIndex >= 0 && alwaysReturns) {
this.currentFunction.flow.set(FlowFlags.RETURNS);
// Binaryen understands that so we don't need a hint
}
return currentBlock;
}
compileThrowStatement(statement: ThrowStatement): ExpressionRef {
// Remember that this branch possibly throws
this.currentFunction.flow.set(FlowFlags.THROWS);
this.currentFunction.flow.set(FlowFlags.POSSIBLY_THROWS);
// FIXME: without try-catch it is safe to assume RETURNS as well for now
this.currentFunction.flow.set(FlowFlags.RETURNS);
// TODO: requires exception-handling spec.
return this.module.createUnreachable();
@ -1176,17 +1214,19 @@ export class Compiler extends DiagnosticEmitter {
// Statements initiate a new branch with its own break context
var label = this.currentFunction.enterBreakContext();
this.currentFunction.flow = this.currentFunction.flow.enterBranch();
this.currentFunction.flow = this.currentFunction.flow.enterBranchOrScope();
var breakLabel = this.currentFunction.flow.breakLabel = "break|" + label;
var continueLabel = this.currentFunction.flow.continueLabel = "continue|" + label;
var body = this.compileStatement(statement.statement);
var alwaysReturns = false && this.currentFunction.flow.is(FlowFlags.RETURNS);
// TODO: evaluate possible always-true conditions
// Switch back to the parent flow
this.currentFunction.flow = this.currentFunction.flow.leaveBranch();
this.currentFunction.flow = this.currentFunction.flow.leaveBranchOrScope();
this.currentFunction.leaveBreakContext();
return this.module.createBlock(breakLabel, [
var expr = this.module.createBlock(breakLabel, [
this.module.createLoop(continueLabel,
this.module.createIf(condition, this.module.createBlock(null, [
body,
@ -1194,6 +1234,15 @@ export class Compiler extends DiagnosticEmitter {
], NativeType.None))
)
], NativeType.None);
// If the loop is guaranteed to run and return, propagate that and append a hint
if (alwaysReturns) {
expr = this.module.createBlock(null, [
expr,
this.module.createUnreachable()
]);
}
return expr;
}
// expressions

View File

@ -70,6 +70,7 @@ export enum DiagnosticCode {
_super_can_only_be_referenced_in_a_derived_class = 2335,
Property_0_does_not_exist_on_type_1 = 2339,
Cannot_invoke_an_expression_whose_type_lacks_a_call_signature_Type_0_has_no_compatible_call_signatures = 2349,
A_function_whose_declared_type_is_not_void_must_return_a_value = 2355,
The_operand_of_an_increment_or_decrement_operator_must_be_a_variable_or_a_property_access = 2357,
The_left_hand_side_of_an_assignment_expression_must_be_a_variable_or_a_property_access = 2364,
_get_and_set_accessor_must_have_the_same_type = 2380,
@ -158,6 +159,7 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
case 2335: return "'super' can only be referenced in a derived class.";
case 2339: return "Property '{0}' does not exist on type '{1}'.";
case 2349: return "Cannot invoke an expression whose type lacks a call signature. Type '{0}' has no compatible call signatures.";
case 2355: return "A function whose declared type is not 'void' must return a value.";
case 2357: return "The operand of an increment or decrement operator must be a variable or a property access.";
case 2364: return "The left-hand side of an assignment expression must be a variable or a property access.";
case 2380: return "'get' and 'set' accessor must have the same type.";

View File

@ -70,6 +70,7 @@
"'super' can only be referenced in a derived class.": 2335,
"Property '{0}' does not exist on type '{1}'.": 2339,
"Cannot invoke an expression whose type lacks a call signature. Type '{0}' has no compatible call signatures.": 2349,
"A function whose declared type is not 'void' must return a value.": 2355,
"The operand of an increment or decrement operator must be a variable or a property access.": 2357,
"The left-hand side of an assignment expression must be a variable or a property access.": 2364,
"'get' and 'set' accessor must have the same type.": 2380,

View File

@ -2123,9 +2123,12 @@ export class Interface extends Class {
/** Control flow flags indicating specific conditions. */
export const enum FlowFlags {
/** No specific conditions. */
NONE = 0,
/** This branch always returns. */
RETURNS = 1 << 0,
THROWS = 1 << 1
/** This branch possibly throws. */
POSSIBLY_THROWS = 1 << 1,
}
/** A control flow evaluator. */
@ -2147,6 +2150,7 @@ export class Flow {
/** Creates the parent flow of the specified function. */
static create(currentFunction: Function): Flow {
var parentFlow = new Flow();
parentFlow.parent = null;
parentFlow.flags = FlowFlags.NONE;
parentFlow.currentFunction = currentFunction;
parentFlow.continueLabel = null;
@ -2162,8 +2166,8 @@ export class Flow {
/** Sets the specified flag or flags. */
set(flag: FlowFlags): void { this.flags |= flag; }
/** Enters a new branch and returns the new flow. */
enterBranch(): Flow {
/** Enters a new branch or scope and returns the new flow. */
enterBranchOrScope(): Flow {
var branchFlow = new Flow();
branchFlow.parent = this;
branchFlow.flags = this.flags;
@ -2173,14 +2177,21 @@ export class Flow {
return branchFlow;
}
/** Leaves the current branch and returns the parent flow. */
leaveBranch(): Flow {
/** Leaves the current branch or scope and returns the parent flow. */
leaveBranchOrScope(): Flow {
var parent = assert(this.parent);
if (this.scopedLocals) {
for (var scopedLocal of this.scopedLocals.values())
this.currentFunction.freeTempLocal(scopedLocal);
this.scopedLocals = null;
}
return assert(this.parent);
// Mark parent as THROWS if any child throws
if (this.is(FlowFlags.POSSIBLY_THROWS))
parent.set(FlowFlags.POSSIBLY_THROWS);
this.continueLabel = null;
this.breakLabel = null;
return parent;
}
/** Adds a new scoped local of the specified name. */
@ -2197,7 +2208,6 @@ export class Flow {
/** Gets the local of the specified name in the current scope. */
getScopedLocal(name: string): Local | null {
// console.log("checking: " + name);
var local: Local | null;
var current: Flow | null = this;
do {
@ -2206,4 +2216,12 @@ export class Flow {
} while (current = current.parent);
return this.currentFunction.locals.get(name);
}
/** Finalizes this flow. Must be the topmost parent flow of the function. */
finalize(): bool {
assert(this.parent == null, "must be the topmost parent flow");
this.continueLabel = null;
this.breakLabel = null;
return this.is(FlowFlags.RETURNS);
}
}

View File

@ -64,7 +64,6 @@ glob.sync(filter, { cwd: __dirname + "/compiler" }).forEach(filename => {
+ "\n[program.exports]\n " + elements(program.exports)
+ "\n;)\n";
var actualOptimized = null;
// var actualInlined = null;
console.log("parse incl. I/O: " + ((parseTime[0] * 1e9 + parseTime[1]) / 1e6).toFixed(3) + "ms / compile: " + ((compileTime[0] * 1e9 + compileTime[1]) / 1e6).toFixed(3) + "ms");
@ -72,7 +71,10 @@ glob.sync(filter, { cwd: __dirname + "/compiler" }).forEach(filename => {
if (module.validate()) {
console.log(chalk.green("validate OK"));
try {
// module.interpret();
// already covered by instantiate below, which is also able to use imports, but doesn't
// provide as much debugging information. might be necessary to remove this one once imports
// are tested more.
module.interpret();
console.log(chalk.green("interpret OK"));
try {
var binary = module.toBinary();

View File

@ -5,6 +5,7 @@
(export "ifThenElse" (func $if/ifThenElse))
(export "ifThen" (func $if/ifThen))
(export "ifThenElseBlock" (func $if/ifThenElse))
(export "ifAlwaysReturns" (func $if/ifAlwaysReturns))
(export "memory" (memory $0))
(start $start)
(func $if/ifThenElse (; 0 ;) (type $ii) (param $0 i32) (result i32)
@ -23,7 +24,16 @@
)
(i32.const 0)
)
(func $start (; 2 ;) (type $v)
(func $if/ifAlwaysReturns (; 2 ;) (type $ii) (param $0 i32) (result i32)
(if
(get_local $0)
(return
(i32.const 1)
)
(unreachable)
)
)
(func $start (; 3 ;) (type $v)
(if
(call $if/ifThenElse
(i32.const 0)

View File

@ -29,3 +29,10 @@ export function ifThenElseBlock(n: i32): bool {
assert(ifThenElseBlock(0) == false);
assert(ifThenElseBlock(1) == true);
export function ifAlwaysReturns(n: i32): bool {
if (n)
return true;
else
throw new Error("error");
}

View File

@ -6,6 +6,7 @@
(export "ifThenElse" (func $if/ifThenElse))
(export "ifThen" (func $if/ifThen))
(export "ifThenElseBlock" (func $if/ifThenElseBlock))
(export "ifAlwaysReturns" (func $if/ifAlwaysReturns))
(export "memory" (memory $0))
(start $start)
(func $if/ifThenElse (; 0 ;) (type $ii) (param $0 i32) (result i32)
@ -47,7 +48,16 @@
)
)
)
(func $start (; 3 ;) (type $v)
(func $if/ifAlwaysReturns (; 3 ;) (type $ii) (param $0 i32) (result i32)
(if
(get_local $0)
(return
(i32.const 1)
)
(unreachable)
)
)
(func $start (; 4 ;) (type $v)
(if
(i32.eqz
(i32.eq
@ -163,8 +173,10 @@
FUNCTION_PROTOTYPE: if/ifThenElse
FUNCTION_PROTOTYPE: if/ifThen
FUNCTION_PROTOTYPE: if/ifThenElseBlock
FUNCTION_PROTOTYPE: if/ifAlwaysReturns
[program.exports]
FUNCTION_PROTOTYPE: if/ifThenElse
FUNCTION_PROTOTYPE: if/ifThen
FUNCTION_PROTOTYPE: if/ifThenElseBlock
FUNCTION_PROTOTYPE: if/ifAlwaysReturns
;)