Some JSInterpreter code cleanup #15744
Conversation
pcardune
commented
Jun 9, 2017
pcardune
commented
- Moved JSInterpreter to lib/tools/jsinterpreter/JSInterpreter.js where it belongs
- Switch everything to es6 class syntax
- Removed use of unnecessary bind calls in favor of arrow functions
- Remove direct access to interpreter stack, putting everything behind accessor functions instead
- Replace var with const and let throughout
| @@ -92,8 +93,11 @@ export function evalWithEvents(apis, events, evalCode = '') { | |||
| // to call, and any arguments. | |||
| const eventLoop = ';while(true){var event=wait();setReturnValue(this[event.name].apply(null,event.args));}'; | |||
|
|
|||
| interpreter = new Interpreter( | |||
| // TODO (pcardune): remove circular dependency | |||
There was a problem hiding this comment.
When will this TODO be addressed?
There was a problem hiding this comment.
I need to move codegen to lib/tools/jsinterpreter. At that point, I will probably move a number of these functions onto CustomMarshalingInterpreter and kill off the circular deps.
apps/src/turtle/turtle.js
Outdated
| this.level[prop] === undefined || | ||
| this.level[prop] === group.defaultValues[prop]))) { | ||
| this.level[prop] === undefined || | ||
| this.level[prop] === group.defaultValues[prop]))) { |
There was a problem hiding this comment.
This whitespace change looks like a mistake.
There was a problem hiding this comment.
yep. I blame my editor which does auto-indenting.
| * @return {boolean} true if program is complete (or an error has occurred). | ||
| */ | ||
| isProgramDone() { | ||
| const topStackFrame = this.interpreter.peekStackFrame(); |
There was a problem hiding this comment.
Possible null reference: Should probably pull the this.interpreter null check in the expression below up into this assignment.
| this.yield(); | ||
| } | ||
| return retVal; | ||
| } |
| "slider to its maximum value)"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
interesting. eslint doesn't seem to catch this.
|
|
||
| if (this.paused) { | ||
| // Store the first call expression stack depth seen while in this step operation: | ||
| if (inUserCode && this.interpreter.peekStackFrame().node.type === "CallExpression") { |
There was a problem hiding this comment.
I still think extracting inUserCallExpression (here and eight lines above) makes sense.
pcardune
force-pushed
the
pcardune-cleanup-jsinterpreter
branch
from
bd16816
to
60acb47
Compare
|
Ok, pushed a new version without the unintentional indentation changes. |
| } | ||
| const topStackFrame = this.interpreter.peekStackFrame(); | ||
| return this.executionError || | ||
| !this.interpreter || |
There was a problem hiding this comment.
Nit: This guard is now redundant.
