Skip to content
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

Remove Interpreter and Testing Refactor #1745

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

leeyi45
Copy link
Contributor

@leeyi45 leeyi45 commented Mar 3, 2025

@martin-henz has indicated that concurrent, concurrency and certain parts of vm can be removed. This PR does just that, though I am not 100% certain I have removed the obsolete components entirely.

Since nothing else relies on the interpreter, this PR also removes it entirely. I have thus managed to refactor our testing utilities. I don't know if that has changed anything, but for the most part all our tests still pass; some error test cases have different error messages now.

Resolves:

@leeyi45
Copy link
Contributor Author

leeyi45 commented Mar 3, 2025

There is remaining work:

  1. I don't know how to refactor the test in src/__tests__/environment.ts to use the cse-machine
  2. I did try removing stdlib/inspector.ts, but the cse-machine relies on it, so although @martin-henz has indicated that it should be removed I have not done that yet.

@leeyi45 leeyi45 mentioned this pull request Mar 3, 2025
@martin-henz
Copy link
Member

There is remaining work:

  1. I don't know how to refactor the test in src/__tests__/environment.ts to use the cse-machine
  2. I did try removing stdlib/inspector.ts, but the cse-machine relies on it, so although @martin-henz has indicated that it should be removed I have not done that yet.

Clarification: The inspector component (src/inspector) has been removed some time ago, which is good. It wasn't maintained and stopped working.

The component stdlib/inspector.ts supported src/inspector by providing the ability to set breakpoints on particular lines and to check if a line has a breakpoint set. This facility was then reused by the cse-machine to run programs to the next breakpoint using > > and < <. So we need to keep stdlib/inspector.ts.

For clarity we should rename it to stdlib/debugging.ts or stdlib/breakpoint.ts.

@leeyi45
Copy link
Contributor Author

leeyi45 commented Mar 4, 2025

There is remaining work:

  1. I don't know how to refactor the test in src/__tests__/environment.ts to use the cse-machine
  2. I did try removing stdlib/inspector.ts, but the cse-machine relies on it, so although @martin-henz has indicated that it should be removed I have not done that yet.

Clarification: The inspector component (src/inspector) has been removed some time ago, which is good. It wasn't maintained and stopped working.

The component stdlib/inspector.ts supported src/inspector by providing the ability to set breakpoints on particular lines and to check if a line has a breakpoint set. This facility was then reused by the cse-machine to run programs to the next breakpoint using > > and < <. So we need to keep stdlib/inspector.ts.

For clarity we should rename it to stdlib/debugging.ts or stdlib/breakpoint.ts.

I've always heavily disliked how it makes use of global variables. Is there something we can do about this?
While we're here, if anyone can provide some insight as to what can be removed from the runtime object on contexts that would be great. Names like debuggerOn don't exactly tell me what's going on unfortunately.

@RichDom2185 RichDom2185 mentioned this pull request Mar 4, 2025
1 task
@RichDom2185 RichDom2185 self-requested a review March 4, 2025 06:14
@leeyi45
Copy link
Contributor Author

leeyi45 commented Mar 4, 2025

There is remaining work:

  1. I don't know how to refactor the test in src/__tests__/environment.ts to use the cse-machine
  2. I did try removing stdlib/inspector.ts, but the cse-machine relies on it, so although @martin-henz has indicated that it should be removed I have not done that yet.

Clarification: The inspector component (src/inspector) has been removed some time ago, which is good. It wasn't maintained and stopped working.

The component stdlib/inspector.ts supported src/inspector by providing the ability to set breakpoints on particular lines and to check if a line has a breakpoint set. This facility was then reused by the cse-machine to run programs to the next breakpoint using > > and < <. So we need to keep stdlib/inspector.ts.

For clarity we should rename it to stdlib/debugging.ts or stdlib/breakpoint.ts.

If only the cse-machine needs it, could we relocate it to the cse-machine?

@martin-henz
Copy link
Member

There is remaining work:

  1. I don't know how to refactor the test in src/__tests__/environment.ts to use the cse-machine
  2. I did try removing stdlib/inspector.ts, but the cse-machine relies on it, so although @martin-henz has indicated that it should be removed I have not done that yet.

Clarification: The inspector component (src/inspector) has been removed some time ago, which is good. It wasn't maintained and stopped working.
The component stdlib/inspector.ts supported src/inspector by providing the ability to set breakpoints on particular lines and to check if a line has a breakpoint set. This facility was then reused by the cse-machine to run programs to the next breakpoint using > > and < <. So we need to keep stdlib/inspector.ts.
For clarity we should rename it to stdlib/debugging.ts or stdlib/breakpoint.ts.

If only the cse-machine needs it, could we relocate it to the cse-machine?

I think other runners could also use break points. It would be a great feature for the stepper too. So in anticipation of that, we could keep the breakpoints feature in stdlib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants