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

Assignment to const results in panic #622

Closed
elasmojs opened this issue Aug 9, 2020 · 5 comments · Fixed by #659
Closed

Assignment to const results in panic #622

elasmojs opened this issue Aug 9, 2020 · 5 comments · Fixed by #659
Assignees
Labels
bug Something isn't working execution Issues or PRs related to code execution
Milestone

Comments

@elasmojs
Copy link
Contributor

elasmojs commented Aug 9, 2020

Hi

Assignment to const causes panic instead of expected TypeError

const number = 42;

try {
    number = 99;
} catch (err) {
    console.log(err);
    // expected output: TypeError: invalid assignment to const `number'
    // Note - error messages will vary depending on browser
}

console.log(number);
// expected output: 42

thanks

@elasmojs elasmojs added the bug Something isn't working label Aug 9, 2020
@54k1
Copy link
Contributor

54k1 commented Aug 11, 2020

There's a TODO mark for many of the methods in environment module. Are they supposed return ResultValue to indicate success or a failure so that the call sites can be notified upon errors and can handle them appropriately?

@HalidOdat could you verify this? Thanks.

@HalidOdat
Copy link
Member

There's a TODO mark for many of the methods in environment module. Are they supposed return ResultValue to indicate success or a failure so that the call sites can be notified upon errors and can handle them appropriately?

@HalidOdat could you verify this? Thanks.

Yes. the environments were set up before exceptions (TypeError, ReferenceError, etc), one way of solving this is to use ResultValue this would require the Interpreter to be passed to every call. Another way is to create a Error enum, that contains, the error type and message and return Result<TheNormalReturntype, Error>, and when we call those function when we need to throw we convert them to a javascript Error (TypeError, RangeError, etc) type. This has the advantage that we dont need to introduce the interpreter to the environments

@54k1
Copy link
Contributor

54k1 commented Aug 11, 2020

Thanks for confirming this and suggesting the approach. Could I take this up?

@HalidOdat
Copy link
Member

Thanks for confirming this and suggesting the approach. Could I take this up?

sure! :)

@HalidOdat HalidOdat added the execution Issues or PRs related to code execution label Aug 11, 2020
@Razican
Copy link
Member

Razican commented Oct 8, 2020

@54k1 let us know if you need any further guidance.

@Razican Razican added this to the v0.11.0 milestone Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working execution Issues or PRs related to code execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants