Introduce main V2 interpreter loop.#2439
Conversation
| @@ -0,0 +1,30 @@ | |||
| package org.mozilla.javascript.interpreterv2.instruction; | |||
There was a problem hiding this comment.
do we have to always add the common header?
There was a problem hiding this comment.
Good question. If we should add it to every file then we've done a bad job of that for a while, even Slot.java doesn't have a licence header.
There was a problem hiding this comment.
We actually are, I got lazy about that, I think that there must be a tool that can fix this for us.
There was a problem hiding this comment.
I believe spotless can enforce this, and I can certainly produce a PR to fill in the missing ones.
| import org.mozilla.javascript.interpreterv2.InstructionFormatter; | ||
|
|
||
| public abstract class JumpInstruction extends Instruction { | ||
| protected int offset; |
There was a problem hiding this comment.
does it make sense to have a protected field and a getter/setter for this internal class
There was a problem hiding this comment.
Good point. I made it private and have changed the upcoming commits to remove direct references.
aardvark179
force-pushed
the
aardvark179-interpreterv2-infra-2
branch
from
dd8cebd to
373de53
Compare
gbrail
left a comment
gbrail
left a comment
There was a problem hiding this comment.
I may be an old fuddy-duddy but I see lots of opportunities for unit tests in here, simple stuff like the "formatter" but even the actual bits of the interpreter too. I will not lose sleep if they are written by AI as long as they have decent coverage. It would make me more confident if I were writing the code myself (or with a tool) to have those additional tests as well.
|
That's reasonable, I'll see what I can do. |
3 participants
This PR is the next part #2426 and introduces the main interpreter v2 interpreter loop, error handling code, and utilitiy functions used by multiple instructions. This will be followed by PRs for the operands and instructions, and finally by the compiler itself.