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

Harmonize exception throwing #515

Merged
merged 1 commit into from Jun 21, 2018

Conversation

Projects
None yet
2 participants
@lahma
Collaborator

lahma commented Jun 19, 2018

This is mostly about cleanup and making exception throwing consistent. Now there's ExceptionHelper that does most of the throwing. This will also add possibility for JIT to inline better as there are no throw statements in function bodies.

@@ -717,13 +722,7 @@ public void PutValue(Reference reference, JsValue value)
else
{
var baseValue = reference.GetBase();
if (!(baseValue is EnvironmentRecord record))

This comment has been minimized.

@sebastienros

sebastienros Jun 21, 2018

Owner

I assume that if there is this test it's because it was in the spec. Maybe there is not code path for it, but I wouldn't remove it, unless you found out it could never be true.

This comment has been minimized.

@lahma

lahma Jun 21, 2018

Collaborator

But as you can see the code right after it does forced cast so the invariant is present. It just changes from ambiguous ArgumentNullException to maybe even better InvalidCastException?

This comment has been minimized.

@sebastienros

sebastienros Jun 21, 2018

Owner

Sorry, I thought it was throwing a JS exception. It's just dotnet one.

@sebastienros sebastienros merged commit e63a022 into sebastienros:dev Jun 21, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

maximburyak added a commit to maximburyak/jint that referenced this pull request Jul 11, 2018

@lahma lahma deleted the lahma:perf/harmonize-exception-throwing branch Jul 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment