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

Builtin methods handle some exceptions #7494

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Aug 3, 2023

Closes #7333

Pull Request Description

For a specific subset of RuntimeException or Error, the invocation builtin method is wrapped in try {...} catch (ErrorType t) {...} and the exception is converted to either panic or a dataflow error. So far, we can handle only org.graalvm.truffle.api.dsl.UnsupportedSpecializationException. All the other exception types are not caught at all.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Akirathan Akirathan self-assigned this Aug 3, 2023
@Akirathan Akirathan marked this pull request as ready for review August 4, 2023 10:57
@Akirathan Akirathan added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 4, 2023
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test. Consider propagating ThreadDeath at least.

@Akirathan
Copy link
Member Author

Trying to catch all the Throwable and convert them to panics or dataflow errors is not a good idea:

  • com.oracle.truffle.api.nodes.ControlFlowException should not be caught at all.
  • Rethrowing Throwable would require incompatible method signature modification.

@Akirathan Akirathan merged commit 52b1189 into develop Aug 7, 2023
25 of 27 checks passed
@Akirathan Akirathan deleted the wip/akirathan/7333-Inject-fallback-specialization-to-builtin-methods branch August 7, 2023 14:42
@Akirathan Akirathan changed the title Builtin methods do not throw any exceptions Builtin methods handle some exceptions Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inject fallback specialization to builtin methods
2 participants