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

[WIP] Fix #98. Convert all Interpreter errors to RuntimeErrors. #107

Merged

Conversation

faultyserver
Copy link
Member

This PR converts all errors raised by the interpreter into rescue-able RuntimeErrors. To accomplish this, the utility method __raise_runtime_error has been added, reducing boilerplate throughout the interpreter source.

When #103 is addressed, most of the errors raised here will change to proper error structures (rather than strings), but that is a future effort.

This PR also moves the NativeLibrary specs into Myst. This is something I've wanted to do for a while, but it's now a requirement to effectively test that RuntimeErrors are being raised.

…to RuntimeErrors, move nativelib specs into Myst.

This commit starts work on converting all errors raised by the interpreter into rescue-able RuntimeErrors. Specifically, this commit addresses all instances of `raise` in the Native Library, using the new `__raise_runtime_error` to raise an appropriate message.

When myst-lang#103 is addressed, these errors will change to proper error structures (rather than strings), but that is a future effort.

Some other instances of `raise` were also converted in the process of this commit.
@faultyserver faultyserver added this to the Next milestone Dec 30, 2017
This commit also fixes two missed `raise`s in String methods to raise RuntimeErrors instead.
This finishes the currently-possible porting of native library specs into the language itself. IO specs cannot be ported because there is currently no way to capture/redirect IO operations.
@faultyserver
Copy link
Member Author

IO specs cannot be ported into the language because there is currently no way to capture/redirect IO operations. These should be the only specs left in the spec/native_lib folder now.

By making MatchError a descendant of RuntimeError, it can be rescued inside the language itself. This is one more step to making the interpreter only raise RuntimeErrors.

The matcher specs needed a fair bit of reworking to play nicely with this change, including a change to `Interpreter#run` that tells the interpreter to allow errors to propogate farther up and be expected by the specs.
…error`.

Everything in the interpreter _should_ now be using `__raise_runtime_error` instead of raising native errors or doing `raise RuntimeError.new`.

There are currently only two exceptions to this:

  - `Myst::Scope#[]` raises a native `IndexError` when a value cannot be found. I don't know how to get this to be a runtime error other than capturing the error at each call site and re-raising appropriately. That might be the best plan.

  - `Myst::Value.from_literal` raises a native string when the given type is not a value type. This could be solved by passing in the interpreter to the method, or, again, capturing at the call site and re-raising. I don't know which I would prefer.
@faultyserver
Copy link
Member Author

From the last commit:

Everything in the interpreter should now be using __raise_runtime_error instead of raising native errors or doing raise RuntimeError.new.

There are currently only two exceptions to this:

  • Myst::Scope#[] raises a native IndexError when a value cannot be found. I don't know how to get this to be a runtime error other than capturing the error at each call site and re-raising appropriately. That might be the best plan.

  • Myst::Value.from_literal raises a native string when the given type is not a value type. This could be solved by passing in the interpreter to the method, or, again, capturing at the call site and re-raising. I don't know which I would prefer.

…rom `Scope#[]`.

It's a bit of a cop-out solution, but the idea is that `IndexError`s raised by `Scope#[]` should not happen anywhere in the interpreter. Any case where the key may not be present should be guarded against appropriately. As such, if the `IndexError` propogates to outside of the Interpreter, it should be considered a bug to be fixed, as the messasge of the error now indicates.
…_literal` raise case.

Any attempt to create a Value from a Node should guard that the Node is a Literal type. Attempting to create a Value from a non-literal type should be considered an Interpreter bug and addressed as such.
@faultyserver
Copy link
Member Author

Kinda copped out with those last two, but I think any case where they come up should be treated as an interpreter bug. Trying to solve them by tossing the interpreter around feels heavy, slow, and poorly-designed, so I'd rather have an implicit guarantee there than muddy things up to make it explicit.

However, any instance where a non-runtime error is raised will now be considered an Interpreter Bug. Before this PR that was not possible, because native lib methods would raise native errors, but that part has been addressed, and all other happy-path parts of the interpreter have also been modified to raise RuntimeErrors instead.

With that, I'm going to merge this :)

@faultyserver faultyserver merged commit 0459d2a into myst-lang:master Jan 6, 2018
@faultyserver faultyserver deleted the fix/98/use_runtime_exceptions branch January 6, 2018 05:40
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.

1 participant