Skip to content

Remove src/interpreter/interpreter.ts #1476

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

Closed
martin-henz opened this issue Sep 3, 2023 · 4 comments
Closed

Remove src/interpreter/interpreter.ts #1476

martin-henz opened this issue Sep 3, 2023 · 4 comments
Assignees
Labels
Proposal Proposing a feature, please discuss

Comments

@martin-henz
Copy link
Member

martin-henz commented Sep 3, 2023

I think src/ec-evaluator is replacing src/interpreter/interpreter.ts, so the latter is dead code.

Background: The interpreter used to be the workhorse implementation in SA, prior to the transpiler. After the transpiler was introduced, it became the implementation behind the environment visualizer. Now the environment visualizer uses ec-evaluator, which means that the interpreter is unemployed.

I suggest we should remove src/interpreter/interpreter.ts from js-slang, and leave some breadcrumbs in the wiki. It is bound to decay over time and cause extra work.

See also #1695

@martin-henz martin-henz added the Proposal Proposing a feature, please discuss label Sep 3, 2023
@martin-henz martin-henz changed the title Remove src/interpreter Remove src/interpreter/interpreter.ts Sep 3, 2023
@RichDom2185
Copy link
Member

I think we should proceed with the removal. The only references to src/interpreter/interpreter.ts are as follows:

  • import { apply } from './interpreter'
    const closureToJS = (value: Closure, context: Context, klass: string) => {
    function DummyClass(this: Closure) {
    const args: Value[] = Array.prototype.slice.call(arguments)
    const gen = apply(context, value, args, callExpression(identifier(klass), args), this)

  • function runInterpreter(program: es.Program, context: Context, options: IOptions): Promise<Result> {
    let it = evaluate(program, context, true, true)
    let scheduler: Scheduler
    if (context.variant === Variant.NON_DET) {
    it = nonDetEvaluate(program, context)
    scheduler = new NonDetScheduler()
    } else if (options.scheduler === 'async') {
    scheduler = new AsyncScheduler()
    } else {
    scheduler = new PreemptiveScheduler(options.steps)
    }
    return scheduler.run(it, context)
    }

  • import { createBlockEnvironment } from '../interpreter/interpreter'

    Perhaps we can just replace this with

    import { createBlockEnvironment } from '../ec-evaluator/utils'

How are would it be to completely remove it?

@martin-henz
Copy link
Member Author

I think we should completely remove it. It's a confusing legacy component at this point. Where are these "mocks" used?

@RichDom2185
Copy link
Member

I think we should completely remove it. It's a confusing legacy component at this point. Where are these "mocks" used?

The mocks in the last bullet point are used in test files. I'll make a PR with the necessary changes.

@leeyi45
Copy link
Contributor

leeyi45 commented Apr 24, 2025

Resolved by #1745

@leeyi45 leeyi45 closed this as completed Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposing a feature, please discuss
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants