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 exc_caught from mrb_vm_exec() #6260

Merged
merged 1 commit into from May 6, 2024

Conversation

dearblue
Copy link
Contributor

@dearblue dearblue commented May 5, 2024

Only go to exception handling if mrb->exc is non-null.

This may cause some compatibility problems, but I doubt that it is necessary to maintain that compatibility.
Here is how I see the incompatibility with the change at this time:

  • If mrb->exc is non-null and mrb_vm_exec() is called, an exception will be thrown immediately.
  • If MRB_THROW() is used while mrb->exc is NULL, it will not go to exception handling.

Only go to exception handling if `mrb->exc` is non-null.

This may cause some compatibility problems, but I doubt that it is necessary to maintain that compatibility.
Here is how I see the incompatibility with the change at this time:
  - If `mrb->exc` is non-null and `mrb_vm_exec()` is called, an exception will be thrown immediately.
  - If `MRB_THROW()` is used while `mrb->exc` is `NULL`, it will not go to exception handling.
@dearblue dearblue requested a review from matz as a code owner May 5, 2024 12:39
@github-actions github-actions bot added the core label May 5, 2024
@dearblue
Copy link
Contributor Author

dearblue commented May 5, 2024

As a note, write down what I am thinking further as a change in behavior.

If mrb->exc is non-null and mrb_vm_exec() is called, an exception will be thrown immediately.

Put mrb->exc = NULL before the RETRY_TRY_BLOCK label.
In principle, mrb->exc is kept NULL in VM.
Taking that idea further, it might not hurt to remove the first if (mrb->exc) condition, such as OP_RETURN.
However, mrb_funcall_with_block() and mrb_yield_argv() may need similar mrb->exc checks before and after C function calls.

If MRB_THROW() is used while mrb->exc is NULL, it will not go to exception handling.

Put mrb->exc = mrb_exc_new_lit(mrb, E_RUNTIME_ERROR, “”) before goto RETRY_TRY_BLOCK.

@matz matz merged commit f975794 into mruby:master May 6, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants