Merge pull request #2240 from LLK/revert-2145-raise-params

Revert "Raise params to the next frame when pushing"
This commit is contained in:
Karishma Chadha 2019-07-22 13:30:28 -04:00 committed by GitHub
commit f6f97ef3a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 163 additions and 268 deletions

View file

@ -13,43 +13,46 @@ class Scratch3ProcedureBlocks {
*/
getPrimitives () {
return {
// procedures_definition is the top block of a procedure but has no
// effect of its own.
procedures_definition: null,
procedures_definition: this.definition,
procedures_call: this.call,
argument_reporter_string_number: this.argumentReporterStringNumber,
argument_reporter_boolean: this.argumentReporterBoolean
};
}
definition () {
// No-op: execute the blocks.
}
call (args, util) {
const procedureCode = args.mutation.proccode;
const paramNamesIdsAndDefaults = util.getProcedureParamNamesIdsAndDefaults(procedureCode);
if (!util.stackFrame.executed) {
const procedureCode = args.mutation.proccode;
const paramNamesIdsAndDefaults = util.getProcedureParamNamesIdsAndDefaults(procedureCode);
// If null, procedure could not be found, which can happen if custom
// block is dragged between sprites without the definition.
// Match Scratch 2.0 behavior and noop.
if (paramNamesIdsAndDefaults === null) {
return;
}
const [paramNames, paramIds, paramDefaults] = paramNamesIdsAndDefaults;
util.startProcedure(procedureCode);
// Initialize params for the current stackFrame to {}, even if the procedure does
// not take any arguments. This is so that `getParam` down the line does not look
// at earlier stack frames for the values of a given parameter (#1729)
util.initParams();
for (let i = 0; i < paramIds.length; i++) {
if (args.hasOwnProperty(paramIds[i])) {
util.pushParam(paramNames[i], args[paramIds[i]]);
} else {
util.pushParam(paramNames[i], paramDefaults[i]);
// If null, procedure could not be found, which can happen if custom
// block is dragged between sprites without the definition.
// Match Scratch 2.0 behavior and noop.
if (paramNamesIdsAndDefaults === null) {
return;
}
}
const [paramNames, paramIds, paramDefaults] = paramNamesIdsAndDefaults;
// Initialize params for the current stackFrame to {}, even if the procedure does
// not take any arguments. This is so that `getParam` down the line does not look
// at earlier stack frames for the values of a given parameter (#1729)
util.initParams();
for (let i = 0; i < paramIds.length; i++) {
if (args.hasOwnProperty(paramIds[i])) {
util.pushParam(paramNames[i], args[paramIds[i]]);
} else {
util.pushParam(paramNames[i], paramDefaults[i]);
}
}
util.stackFrame.executed = true;
util.startProcedure(procedureCode);
}
}
argumentReporterStringNumber (args, util) {

View file

@ -60,7 +60,11 @@ class BlockUtility {
* @type {object}
*/
get stackFrame () {
return this.thread.getExecutionContext();
const frame = this.thread.peekStackFrame();
if (frame.executionContext === null) {
frame.executionContext = {};
}
return frame.executionContext;
}
/**

View file

@ -279,7 +279,7 @@ class BlockCached {
// Assign opcode isHat and blockFunction data to avoid dynamic lookups.
this._isHat = runtime.getIsHat(opcode);
this._blockFunction = runtime.getOpcodeFunction(opcode);
this._definedBlockFunction = typeof this._blockFunction === 'function';
this._definedBlockFunction = typeof this._blockFunction !== 'undefined';
// Store the current shadow value if there is a shadow value.
const fieldKeys = Object.keys(fields);
@ -385,6 +385,7 @@ const execute = function (sequencer, thread) {
// Current block to execute is the one on the top of the stack.
const currentBlockId = thread.peekStack();
const currentStackFrame = thread.peekStackFrame();
let blockContainer = thread.blockContainer;
let blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId, BlockCached);
@ -403,8 +404,8 @@ const execute = function (sequencer, thread) {
const length = ops.length;
let i = 0;
if (thread.reported !== null) {
const reported = thread.reported;
if (currentStackFrame.reported !== null) {
const reported = currentStackFrame.reported;
// Reinstate all the previous values.
for (; i < reported.length; i++) {
const {opCached: oldOpCached, inputValue} = reported[i];
@ -440,7 +441,7 @@ const execute = function (sequencer, thread) {
}
// The reporting block must exist and must be the next one in the sequence of operations.
if (thread.justReported !== null && ops[i] && ops[i].id === thread.reportingBlockId) {
if (thread.justReported !== null && ops[i] && ops[i].id === currentStackFrame.reporting) {
const opCached = ops[i];
const inputValue = thread.justReported;
@ -461,8 +462,8 @@ const execute = function (sequencer, thread) {
i += 1;
}
thread.reportingBlockId = null;
thread.reported = null;
currentStackFrame.reporting = null;
currentStackFrame.reported = null;
}
for (; i < length; i++) {
@ -517,8 +518,8 @@ const execute = function (sequencer, thread) {
// operation if it is promise waiting will set its parent value at
// that time.
thread.justReported = null;
thread.reportingBlockId = ops[i].id;
thread.reported = ops.slice(0, i).map(reportedCached => {
currentStackFrame.reporting = ops[i].id;
currentStackFrame.reported = ops.slice(0, i).map(reportedCached => {
const inputName = reportedCached._parentKey;
const reportedValues = reportedCached._parentValues;

View file

@ -724,7 +724,7 @@ class Runtime extends EventEmitter {
if (packageObject.getPrimitives) {
const packagePrimitives = packageObject.getPrimitives();
for (const op in packagePrimitives) {
if (typeof packagePrimitives[op] === 'function') {
if (packagePrimitives.hasOwnProperty(op)) {
this._primitives[op] =
packagePrimitives[op].bind(packageObject);
}
@ -1567,7 +1567,7 @@ class Runtime extends EventEmitter {
isActiveThread (thread) {
return (
(
thread.stackFrame !== null &&
thread.stack.length > 0 &&
thread.status !== Thread.STATUS_DONE) &&
this.threads.indexOf(thread) > -1);
}
@ -1744,20 +1744,11 @@ class Runtime extends EventEmitter {
// Start the thread with this top block.
newThreads.push(this._pushThread(topBlockId, target));
}, optTarget);
// For compatibility with Scratch 2, edge triggered hats need to be
// processed before threads are stepped. See ScratchRuntime.as for
// original implementation.
//
// TODO: Move the execute call to sequencer. Maybe in a method call
// stepHat or stepOne.
// For compatibility with Scratch 2, edge triggered hats need to be processed before
// threads are stepped. See ScratchRuntime.as for original implementation
newThreads.forEach(thread => {
execute(this.sequencer, thread);
if (thread.status !== Thread.STATUS_DONE) {
thread.goToNextBlock();
if (thread.stackFrame === null) {
this.sequencer.retireThread(thread);
}
}
thread.goToNextBlock();
});
return newThreads;
}

View file

@ -103,7 +103,7 @@ class Sequencer {
for (let i = 0; i < threads.length; i++) {
const activeThread = this.activeThread = threads[i];
// Check if the thread is done so it is not executed.
if (activeThread.stackFrame === null ||
if (activeThread.stack.length === 0 ||
activeThread.status === Thread.STATUS_DONE) {
// Finished with this thread.
stoppedThread = true;
@ -137,7 +137,7 @@ class Sequencer {
}
// Check if the thread completed while it just stepped to make
// sure we remove it before the next iteration of all threads.
if (activeThread.stackFrame === null ||
if (activeThread.stack.length === 0 ||
activeThread.status === Thread.STATUS_DONE) {
// Finished with this thread.
stoppedThread = true;
@ -156,7 +156,7 @@ class Sequencer {
let nextActiveThread = 0;
for (let i = 0; i < this.runtime.threads.length; i++) {
const thread = this.runtime.threads[i];
if (thread.stackFrame !== null &&
if (thread.stack.length !== 0 &&
thread.status !== Thread.STATUS_DONE) {
this.runtime.threads[nextActiveThread] = thread;
nextActiveThread++;
@ -184,7 +184,7 @@ class Sequencer {
thread.popStack();
// Did the null follow a hat block?
if (thread.peekStackFrame() === null) {
if (thread.stack.length === 0) {
thread.status = Thread.STATUS_DONE;
return;
}
@ -248,7 +248,7 @@ class Sequencer {
while (!thread.peekStack()) {
thread.popStack();
if (thread.stackFrame === null) {
if (thread.stack.length === 0) {
// No more stack to run!
thread.status = Thread.STATUS_DONE;
return;
@ -299,11 +299,7 @@ class Sequencer {
currentBlockId,
branchNum
);
if (isLoop) {
const stackFrame = thread.peekStackFrame();
stackFrame.needsReset = true;
stackFrame.isLoop = true;
}
thread.peekStackFrame().isLoop = isLoop;
if (branchId) {
// Push branch ID to the thread's stack.
thread.pushStack(branchId);
@ -365,9 +361,7 @@ class Sequencer {
*/
retireThread (thread) {
thread.stack = [];
thread.pointer = null;
thread.stackFrames = [];
thread.stackFrame = null;
thread.stackFrame = [];
thread.requestScriptGlowInFrame = false;
thread.status = Thread.STATUS_DONE;
}

View file

@ -5,23 +5,14 @@
const _stackFrameFreeList = [];
/**
* Default params object for stack frames outside of a procedure.
*
* StackFrame.params uses a null prototype object. It does not have Object
* methods like hasOwnProperty. With a null prototype
* `typeof params[key] !== 'undefined'` has similar behaviour to hasOwnProperty.
* @type {object}
*/
const defaultParams = Object.create(null);
/**
* A frame used for each level of the stack. A general purpose place to store a
* bunch of execution contexts and parameters.
* A frame used for each level of the stack. A general purpose
* place to store a bunch of execution context and parameters
* @param {boolean} warpMode Whether this level of the stack is warping
* @constructor
* @private
*/
class _StackFrame {
constructor () {
constructor (warpMode) {
/**
* Whether this level of the stack is a loop.
* @type {boolean}
@ -29,62 +20,90 @@ class _StackFrame {
this.isLoop = false;
/**
* Whether this level is in warp mode. Set to true by the sequencer for
* some procedures.
*
* After being set to true at the beginning of a procedure a thread
* will be in warpMode until it pops a stack frame to reveal one that
* is not in warpMode. Either this value is always false for a stack
* frame or always true after a procedure sets it.
* Whether this level is in warp mode. Is set by some legacy blocks and
* "turbo mode"
* @type {boolean}
*/
this.warpMode = false;
this.warpMode = warpMode;
/**
* Reported value from just executed block.
* @type {Any}
*/
this.justReported = null;
/**
* The active block that is waiting on a promise.
* @type {string}
*/
this.reporting = '';
/**
* Persists reported inputs during async block.
* @type {Object}
*/
this.reported = null;
/**
* Name of waiting reporter.
* @type {string}
*/
this.waitingReporter = null;
/**
* Procedure parameters.
*
* After being set by a procedure these values do not change and they
* will be copied to deeper stack frames.
* @type {Object}
*/
this.params = defaultParams;
this.params = null;
/**
* A context passed to block implementations.
* @type {Object}
*/
this.executionContext = {};
/**
* Has this frame changed and need a reset?
* @type {boolean}
*/
this.needsReset = false;
this.executionContext = null;
}
/**
* Reset some properties of the frame to default values. Used to recycle.
* Reset all properties of the frame to pristine null and false states.
* Used to recycle.
* @return {_StackFrame} this
*/
reset () {
this.isLoop = false;
this.executionContext = {};
this.needsReset = false;
this.warpMode = false;
this.justReported = null;
this.reported = null;
this.waitingReporter = null;
this.params = null;
this.executionContext = null;
return this;
}
/**
* Create or recycle a stack frame object.
* @param {_StackFrame} parent Parent frame to copy "immutable" values.
* @returns {_StackFrame} The clean stack frame with correct warpMode
* setting.
* Reuse an active stack frame in the stack.
* @param {?boolean} warpMode defaults to current warpMode
* @returns {_StackFrame} this
*/
static create (parent) {
const stackFrame = _stackFrameFreeList.pop() || new _StackFrame();
stackFrame.warpMode = parent.warpMode;
stackFrame.params = parent.params;
return stackFrame;
reuse (warpMode = this.warpMode) {
this.reset();
this.warpMode = Boolean(warpMode);
return this;
}
/**
* Create or recycle a stack frame object.
* @param {boolean} warpMode Enable warpMode on this frame.
* @returns {_StackFrame} The clean stack frame with correct warpMode setting.
*/
static create (warpMode) {
const stackFrame = _stackFrameFreeList.pop();
if (typeof stackFrame !== 'undefined') {
stackFrame.warpMode = Boolean(warpMode);
return stackFrame;
}
return new _StackFrame(warpMode);
}
/**
@ -92,22 +111,12 @@ class _StackFrame {
* @param {_StackFrame} stackFrame The frame to reset and recycle.
*/
static release (stackFrame) {
if (stackFrame !== null) {
_stackFrameFreeList.push(
stackFrame.needsReset ? stackFrame.reset() : stackFrame
);
if (typeof stackFrame !== 'undefined') {
_stackFrameFreeList.push(stackFrame.reset());
}
}
}
/**
* The initial stack frame for all threads. A call to pushStack will create the
* first to be used frame for a thread. That first frame will use the initial
* values from initialStackFrame.
* @type {_StackFrame}
*/
const initialStackFrame = new _StackFrame();
/**
* A thread is a running stack context and all the metadata needed.
* @param {?string} firstBlock First block to execute in the thread.
@ -128,25 +137,12 @@ class Thread {
*/
this.stack = [];
/**
* The "instruction" pointer the thread is currently at. This
* determines what block is executed.
* @type {string}
*/
this.pointer = null;
/**
* Stack frames for the thread. Store metadata for the executing blocks.
* @type {Array.<_StackFrame>}
*/
this.stackFrames = [];
/**
* The current stack frame that goes along with the pointer.
* @type {_StackFrame}
*/
this.stackFrame = null;
/**
* Status of the thread, one of three states (below)
* @type {number}
@ -190,25 +186,7 @@ class Thread {
*/
this.warpTimer = null;
/**
* The value just reported by a promise.
* @type {*}
*/
this.justReported = null;
/**
* The id of the block that we will report the promise resolved value
* for.
* @type {string}
*/
this.reportingBlockId = null;
/**
* The already reported values in a sequence of blocks to restore when
* the awaited promise resolves.
* @type {Array.<*>}
*/
this.reported = null;
}
/**
@ -261,16 +239,12 @@ class Thread {
* @param {string} blockId Block ID to push to stack.
*/
pushStack (blockId) {
if (this.stackFrame === null) {
this.pointer = blockId;
this.stackFrame = _StackFrame.create(initialStackFrame);
} else {
this.stack.push(this.pointer);
this.pointer = blockId;
const parent = this.stackFrame;
this.stackFrames.push(parent);
this.stackFrame = _StackFrame.create(parent);
this.stack.push(blockId);
// Push an empty stack frame, if we need one.
// Might not, if we just popped the stack.
if (this.stack.length > this.stackFrames.length) {
const parent = this.stackFrames[this.stackFrames.length - 1];
this.stackFrames.push(_StackFrame.create(typeof parent !== 'undefined' && parent.warpMode));
}
}
@ -280,47 +254,34 @@ class Thread {
* @param {string} blockId Block ID to push to stack.
*/
reuseStackForNextBlock (blockId) {
this.pointer = blockId;
if (this.stackFrame.needsReset) this.stackFrame.reset();
this.stack[this.stack.length - 1] = blockId;
this.stackFrames[this.stackFrames.length - 1].reuse();
}
/**
* Move the instruction pointer to the last value before this stack of
* blocks was pushed and executed.
* @return {?string} Block ID popped from the stack.
* Pop last block on the stack and its stack frame.
* @return {string} Block ID popped from the stack.
*/
popStack () {
const lastPointer = this.pointer;
this.pointer = this.stack.pop() || null;
_StackFrame.release(this.stackFrame);
this.stackFrame = this.stackFrames.pop() || null;
return lastPointer;
_StackFrame.release(this.stackFrames.pop());
return this.stack.pop();
}
/**
* Move the instruction pointer to the last procedure call block and resume
* execution there or to the end of this thread and stop executing this
* thread.
* Pop back down the stack frame until we hit a procedure call or the stack frame is emptied
*/
stopThisScript () {
let blockID = this.peekStack();
while (blockID !== null) {
const block = this.target.blocks.getBlock(blockID);
if (typeof block !== 'undefined' && block.opcode === 'procedures_call') {
// If we do not push this null, Sequencer will not step to the
// next block and will erroneously call the procedure a second
// time.
//
// By pushing null, Sequencer will treat it as the end of a
// substack, pop the stack and step to the next block.
this.pushStack(null);
break;
}
this.popStack();
blockID = this.peekStack();
}
if (this.stackFrame === null) {
if (this.stack.length === 0) {
// Clean up!
this.requestScriptGlowInFrame = false;
this.status = Thread.STATUS_DONE;
@ -332,7 +293,7 @@ class Thread {
* @return {?string} Block ID on top of stack.
*/
peekStack () {
return this.pointer;
return this.stack.length > 0 ? this.stack[this.stack.length - 1] : null;
}
@ -341,7 +302,7 @@ class Thread {
* @return {?object} Last stack frame stored on this thread.
*/
peekStackFrame () {
return this.stackFrame;
return this.stackFrames.length > 0 ? this.stackFrames[this.stackFrames.length - 1] : null;
}
/**
@ -349,7 +310,7 @@ class Thread {
* @return {?object} Second to last stack frame stored on this thread.
*/
peekParentStackFrame () {
return this.stackFrames.length > 0 ? this.stackFrames[this.stackFrames.length - 1] : null;
return this.stackFrames.length > 1 ? this.stackFrames[this.stackFrames.length - 2] : null;
}
/**
@ -360,22 +321,14 @@ class Thread {
this.justReported = typeof value === 'undefined' ? null : value;
}
/**
* Return an execution context for a block to use.
* @returns {object} the execution context
*/
getExecutionContext () {
const frame = this.stackFrame;
frame.needsReset = true;
return frame.executionContext;
}
/**
* Initialize procedure parameters on this stack frame.
*/
initParams () {
const stackFrame = this.stackFrame;
stackFrame.params = Object.create(null);
const stackFrame = this.peekStackFrame();
if (stackFrame.params === null) {
stackFrame.params = {};
}
}
/**
@ -385,7 +338,7 @@ class Thread {
* @param {*} value Value to set for parameter.
*/
pushParam (paramName, value) {
const stackFrame = this.stackFrame;
const stackFrame = this.peekStackFrame();
stackFrame.params[paramName] = value;
}
@ -395,9 +348,15 @@ class Thread {
* @return {*} value Value for parameter.
*/
getParam (paramName) {
const stackFrame = this.stackFrame;
if (typeof stackFrame.params[paramName] !== 'undefined') {
return stackFrame.params[paramName];
for (let i = this.stackFrames.length - 1; i >= 0; i--) {
const frame = this.stackFrames[i];
if (frame.params === null) {
continue;
}
if (frame.params.hasOwnProperty(paramName)) {
return frame.params[paramName];
}
return null;
}
return null;
}
@ -417,26 +376,26 @@ class Thread {
* where execution proceeds from one block to the next.
*/
goToNextBlock () {
const nextBlockId = this.target.blocks.getNextBlock(this.pointer);
const nextBlockId = this.target.blocks.getNextBlock(this.peekStack());
this.reuseStackForNextBlock(nextBlockId);
}
/**
* Attempt to determine whether a procedure call is recursive, by examining
* the stack.
* Attempt to determine whether a procedure call is recursive,
* by examining the stack.
* @param {!string} procedureCode Procedure code of procedure being called.
* @return {boolean} True if the call appears recursive.
*/
isRecursiveCall (procedureCode) {
const stackHeight = this.stack.length;
// Limit the number of stack levels that are examined for procedures.
const stackBottom = Math.max(stackHeight - 5, 0);
for (let i = stackHeight - 1; i >= stackBottom; i--) {
let callCount = 5; // Max number of enclosing procedure calls to examine.
const sp = this.stack.length - 1;
for (let i = sp - 1; i >= 0; i--) {
const block = this.target.blocks.getBlock(this.stack[i]);
if (block.opcode === 'procedures_call' &&
block.mutation.proccode === procedureCode) {
return true;
}
if (--callCount < 0) return false;
}
return false;
}

View file

@ -120,40 +120,16 @@ test('stepToBranch', t => {
const r = new Runtime();
const s = new Sequencer(r);
const th = generateThread(r);
// Push substack 2 (null).
s.stepToBranch(th, 2, false);
t.strictEquals(th.peekStack(), null);
th.popStack();
t.strictEquals(th.peekStackFrame().isLoop, false);
// Push substack 1 (null).
s.stepToBranch(th, 1, false);
t.strictEquals(th.peekStack(), null);
th.popStack();
t.strictEquals(th.peekStackFrame().isLoop, false);
// Push loop substack (null).
s.stepToBranch(th, 1, true);
t.strictEquals(th.peekStack(), null);
th.popStack();
t.strictEquals(th.peekStackFrame().isLoop, true);
// isLoop resets when thread goes to next block.
th.goToNextBlock();
t.strictEquals(th.peekStackFrame().isLoop, false);
th.popStack();
// Push substack 1 (not null).
s.stepToBranch(th, 1, false);
t.notEquals(th.peekStack(), null);
th.popStack();
t.strictEquals(th.peekStackFrame().isLoop, false);
// Push loop substack (not null).
s.stepToBranch(th, 1, true);
t.notEquals(th.peekStack(), null);
th.popStack();
t.strictEquals(th.peekStackFrame().isLoop, true);
// isLoop resets when thread goes to next block.
th.goToNextBlock();
t.strictEquals(th.peekStackFrame().isLoop, false);
t.end();
});
@ -161,7 +137,7 @@ test('retireThread', t => {
const r = new Runtime();
const s = new Sequencer(r);
const th = generateThread(r);
t.strictEquals(th.stack.length, 11);
t.strictEquals(th.stack.length, 12);
s.retireThread(th);
t.strictEquals(th.stack.length, 0);
t.strictEquals(th.status, Thread.STATUS_DONE);

View file

@ -40,7 +40,7 @@ test('popStack', t => {
const th = new Thread('arbitraryString');
th.pushStack('arbitraryString');
t.strictEquals(th.popStack(), 'arbitraryString');
t.strictEquals(th.popStack(), null);
t.strictEquals(th.popStack(), undefined);
t.end();
});
@ -209,54 +209,21 @@ test('stopThisScript', t => {
x: 0,
y: 0
};
const block3 = {fields: Object,
id: 'thirdString',
inputs: Object,
STEPS: Object,
block: 'fakeBlock',
name: 'STEPS',
next: null,
opcode: 'procedures_definition',
mutation: {proccode: 'fakeCode'},
parent: null,
shadow: false,
topLevel: true,
x: 0,
y: 0
};
rt.blocks.createBlock(block1);
rt.blocks.createBlock(block2);
rt.blocks.createBlock(block3);
th.target = rt;
th.stopThisScript();
t.strictEquals(th.peekStack(), null);
t.strictEquals(th.peekStackFrame(), null);
th.pushStack('arbitraryString');
t.strictEquals(th.peekStack(), 'arbitraryString');
t.notEqual(th.peekStackFrame(), null);
th.stopThisScript();
t.strictEquals(th.peekStack(), null);
t.strictEquals(th.peekStackFrame(), null);
th.pushStack('arbitraryString');
th.pushStack('secondString');
th.stopThisScript();
t.strictEquals(th.peekStack(), null);
t.same(th.stack, ['arbitraryString', 'secondString']);
t.notEqual(th.peekStackFrame(), null);
while (th.peekStackFrame()) th.popStack();
th.pushStack('arbitraryString');
th.pushStack('secondString');
th.pushStack('thirdString');
th.stopThisScript();
t.strictEquals(th.peekStack(), null);
t.same(th.stack, ['arbitraryString', 'secondString']);
t.notEqual(th.peekStackFrame(), null);
t.strictEquals(th.peekStack(), 'secondString');
t.end();
});