Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the interpreter to use Completion Record types #1540

Closed
5 tasks
jedel1043 opened this issue Aug 31, 2021 · 0 comments
Closed
5 tasks

Refactor the interpreter to use Completion Record types #1540

jedel1043 opened this issue Aug 31, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@jedel1043
Copy link
Member

jedel1043 commented Aug 31, 2021

Right now when we're interpreting the AST we utilize JsResult as a wrapper for any error that originates on execution. This has its advantages; we can propagate type errors and throw errors with ? and return any value generated from a native function with ease. However, #672 showed us that having no distinction between a value returned from a native function and a value returned from AST interpretation is suboptimal.

The spec solves this problem with Completion Records, which are a way to separate values generated from native functions and values generated from the execution of Javascript code.
This technique also allows us to eliminate InterpreterState from the Context, essentially removing state that we needed to maintain by hand, like in:

context
.executor()
.set_current_state(InterpreterState::Return);
result

or

context
.executor()
.set_current_state(InterpreterState::Executing);

To implement CompletionRecords we would need to make a big refactor to the AST. Here's a non-exhaustive list in an arbitrary order of the required tasks:

  • Implement a CompletionRecord struct type with its associated methods from the spec
  • Make the associated function run from the Executable trait return a JsResult<CompletionRecord>.
  • Extensively check the spec to obtain the CompletionRecord of every AST node evaluation.
  • Eliminate InterpreterState from the Context type. (We can reuse the type as the [[Type]] field of a CompletionRecord)
  • Solve any bugs originated from the refactor.
@jedel1043 jedel1043 added the enhancement New feature or request label Aug 31, 2021
@boa-dev boa-dev locked and limited conversation to collaborators Aug 31, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant