Fix some of the finer details with regards to Completions

JS completion records have some finer details that can only be
seen when using eval(), where the value of the completion record
becomes important.

Fix most of those cases to be compliant with the spec.

Change-Id: I0c8105a8e778de7be3aea151d1bd64243aea067c
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

3 files changed, 121 insertions, 11 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 9337f6c625..2aa1cef451 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -352,21 +352,96 @@ void Codegen::program(Program *ast)
     }
 }
 
+enum class CompletionState {
+    Empty,
+    EmptyAbrupt,
+    NonEmpty
+};
+
+static CompletionState completionState(StatementList *list)
+{
+    for (StatementList *it = list; it; it = it->next) {
+        if (it->statement->kind == Statement::Kind_BreakStatement ||
+            it->statement->kind == Statement::Kind_ContinueStatement)
+            return CompletionState::EmptyAbrupt;
+        if (it->statement->kind == Statement::Kind_EmptyStatement ||
+            it->statement->kind == Statement::Kind_VariableDeclaration ||
+            it->statement->kind == Statement::Kind_FunctionDeclaration)
+            continue;
+        if (it->statement->kind == Statement::Kind_Block) {
+            CompletionState subState = completionState(static_cast<Block *>(it->statement)->statements);
+            if (subState != CompletionState::Empty)
+                return subState;
+            continue;
+        }
+        return CompletionState::NonEmpty;
+    }
+    return CompletionState::Empty;
+}
+
+static Node *completionStatement(StatementList *list)
+{
+    Node *completionStatement = nullptr;
+    for (StatementList *it = list; it; it = it->next) {
+        if (it->statement->kind == Statement::Kind_BreakStatement ||
+            it->statement->kind == Statement::Kind_ContinueStatement)
+            return completionStatement;
+        if (it->statement->kind == Statement::Kind_ThrowStatement ||
+            it->statement->kind == Statement::Kind_ReturnStatement)
+            return it->statement;
+        if (it->statement->kind == Statement::Kind_EmptyStatement ||
+            it->statement->kind == Statement::Kind_VariableStatement ||
+            it->statement->kind == Statement::Kind_FunctionDeclaration)
+            continue;
+        if (it->statement->kind == Statement::Kind_Block) {
+            CompletionState state = completionState(static_cast<Block *>(it->statement)->statements);
+            switch (state) {
+            case CompletionState::Empty:
+                continue;
+            case CompletionState::EmptyAbrupt:
+                return it->statement;
+            case CompletionState::NonEmpty:
+                break;
+            }
+        }
+        completionStatement = it->statement;
+    }
+    return completionStatement;
+}
+
 void Codegen::statementList(StatementList *ast)
 {
+    if (!ast)
+        return;
+
     bool _requiresReturnValue = requiresReturnValue;
-    requiresReturnValue = false;
+    // ### the next line is pessimizing a bit too much, as there are many cases, where the complietion from the break
+    // statement will not be used, but it's at least spec compliant
+    if (!controlFlow || !controlFlow->hasLoop())
+        requiresReturnValue = false;
+
+    Node *needsCompletion = nullptr;
+
+    if (_requiresReturnValue && !requiresReturnValue)
+        needsCompletion = completionStatement(ast);
+
+    if (requiresReturnValue && !needsCompletion && !insideSwitch) {
+        // break or continue is the first real statement, set the return value to undefined
+        Reference::fromConst(this, Encode::undefined()).storeOnStack(_returnAddress);
+    }
+
+    bool _insideSwitch = insideSwitch;
+    insideSwitch = false;
+
     for (StatementList *it = ast; it; it = it->next) {
-        if (!it->next ||
-            it->next->statement->kind == Statement::Kind_BreakStatement ||
-            it->next->statement->kind == Statement::Kind_ContinueStatement ||
-            it->next->statement->kind == Statement::Kind_ReturnStatement)
-                requiresReturnValue = _requiresReturnValue;
+        if (it->statement == needsCompletion)
+            requiresReturnValue = true;
         if (Statement *s = it->statement->statementCast())
             statement(s);
         else
             statement(static_cast<ExpressionNode *>(it->statement));
-        requiresReturnValue = false;
+        if (it->statement == needsCompletion)
+            requiresReturnValue = false;
         if (it->statement->kind == Statement::Kind_ThrowStatement ||
             it->statement->kind == Statement::Kind_BreakStatement ||
             it->statement->kind == Statement::Kind_ContinueStatement ||
@@ -375,6 +450,7 @@ void Codegen::statementList(StatementList *ast)
             break;
     }
     requiresReturnValue = _requiresReturnValue;
+    insideSwitch = _insideSwitch;
 }
 
 void Codegen::variableDeclaration(PatternElement *ast)
@@ -2379,6 +2455,9 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     BytecodeGenerator::Label *savedReturnLabel = _returnLabel;
     _returnLabel = nullptr;
 
+    bool savedFunctionEndsWithReturn = functionEndsWithReturn;
+    functionEndsWithReturn = endsWithReturn(_module, body);
+
     // reserve the js stack frame (Context & js Function & accumulator)
     bytecodeGenerator->newRegisterArray(sizeof(CallData)/sizeof(Value) - 1 + _context->arguments.size());
 
@@ -2442,7 +2521,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     bytecodeGenerator->setLocation(ast->lastSourceLocation());
     _context->emitBlockFooter(this);
 
-    if (_returnLabel || hasError || !endsWithReturn(_module, body)) {
+    if (_returnLabel || hasError || !functionEndsWithReturn) {
         if (_returnLabel)
             _returnLabel->link();
 
@@ -2463,7 +2542,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     static const bool showCode = qEnvironmentVariableIsSet("QV4_SHOW_BYTECODE");
     if (showCode) {
         qDebug() << "=== Bytecode for" << _context->name << "strict mode" << _context->isStrict
-                 << "register count" << _context->registerCountInFunction;
+                 << "register count" << _context->registerCountInFunction << "implicit return" << requiresReturnValue;
         QV4::Moth::dumpBytecode(_context->code, _context->locals.size(), _context->arguments.size(),
                                 _context->line, _context->lineNumberMapping);
         qDebug();
@@ -2476,6 +2555,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     delete _returnLabel;
     _returnLabel = savedReturnLabel;
     controlFlow = savedControlFlow;
+    functionEndsWithReturn = savedFunctionEndsWithReturn;
     _functionContext = savedFunctionContext;
 
     return leaveContext();
@@ -2842,6 +2922,9 @@ bool Codegen::visit(SwitchStatement *ast)
     if (hasError)
         return true;
 
+    if (requiresReturnValue)
+        Reference::fromConst(this, Encode::undefined()).storeOnStack(_returnAddress);
+
     RegisterScope scope(this);
 
     if (ast->block) {
@@ -2889,6 +2972,7 @@ bool Codegen::visit(SwitchStatement *ast)
 
         ControlFlowLoop flow(this, &switchEnd);
 
+        insideSwitch = true;
         for (CaseClauses *it = ast->block->clauses; it; it = it->next) {
             CaseClause *clause = it->clause;
             blockMap[clause].link();
@@ -2909,6 +2993,7 @@ bool Codegen::visit(SwitchStatement *ast)
 
             statementList(clause->statements);
         }
+        insideSwitch = false;
 
         switchEnd.link();
 
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index 697da71480..39309f78ce 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -684,7 +684,9 @@ protected:
     bool _strictMode;
     bool useFastLookups = true;
     bool requiresReturnValue = false;
+    bool insideSwitch = false;
     bool inFormalParameterList = false;
+    bool functionEndsWithReturn = false;
     ControlFlow *controlFlow = nullptr;
 
     bool _fileNameIsUrl;
diff --git a/src/qml/compiler/qv4compilercontrolflow_p.h b/src/qml/compiler/qv4compilercontrolflow_p.h
index 9ab581f015..415a112ec5 100644
--- a/src/qml/compiler/qv4compilercontrolflow_p.h
+++ b/src/qml/compiler/qv4compilercontrolflow_p.h
@@ -119,6 +119,16 @@ struct ControlFlow {
 
     virtual QString label() const { return QString(); }
 
+    bool hasLoop() const {
+        const ControlFlow *flow = this;
+        while (flow) {
+            if (flow->type == Loop)
+                return true;
+            flow = flow->parent;
+        }
+        return false;
+    }
+
 protected:
     virtual BytecodeGenerator::Label getUnwindTarget(UnwindType, const QString & = QString()) {
         return BytecodeGenerator::Label();
@@ -355,7 +365,6 @@ struct ControlFlowFinally : public ControlFlowUnwind
 {
     AST::Finally *finally;
     bool insideFinally = false;
-    int exceptionTemp = -1;
 
     ControlFlowFinally(Codegen *cg, AST::Finally *finally)
         : ControlFlowUnwind(cg, Finally), finally(finally)
@@ -380,9 +389,17 @@ struct ControlFlowFinally : public ControlFlowUnwind
         Codegen::RegisterScope scope(cg);
 
         insideFinally = true;
-        exceptionTemp = generator()->newRegister();
+        int exceptionTemp = generator()->newRegister();
         Instruction::GetException instr;
         generator()->addInstruction(instr);
+        int returnValueTemp = -1;
+        if (cg->requiresReturnValue) {
+            returnValueTemp = generator()->newRegister();
+            Instruction::MoveReg move;
+            move.srcReg = cg->_returnAddress;
+            move.destReg = returnValueTemp;
+            generator()->addInstruction(move);
+        }
         Reference::fromStackSlot(cg, exceptionTemp).storeConsumeAccumulator();
 
         generator()->setUnwindHandler(parentUnwindHandler());
@@ -391,6 +408,12 @@ struct ControlFlowFinally : public ControlFlowUnwind
 
         Reference::fromStackSlot(cg, exceptionTemp).loadInAccumulator();
 
+        if (cg->requiresReturnValue) {
+            Instruction::MoveReg move;
+            move.srcReg = returnValueTemp;
+            move.destReg = cg->_returnAddress;
+            generator()->addInstruction(move);
+        }
         Instruction::SetException setException;
         generator()->addInstruction(setException);