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

avoid abort_on_panic dependency #58

Merged
merged 2 commits into from
Sep 2, 2022
Merged

Conversation

RalfJung
Copy link
Contributor

@RalfJung RalfJung commented Aug 31, 2022

This can be implemented trivially in a 10-line macro, so it does not seem worth the dependency.

abort_on_panic is licensed under CC0 which is falling out of favor and which might be a problem for having Miri depend on libffi-rs.

(I have implemented this from scratch without looking at the abort_on_panic code.)

libffi-rs/src/high/mod.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Contributor Author

RalfJung commented Sep 2, 2022

I don't know what's happening on riscv here, might be a problem that already exist on master?

@yorickpeterse yorickpeterse merged commit c219779 into tov:master Sep 2, 2022
@yorickpeterse
Copy link
Collaborator

@RalfJung The riscv errors are unrelated and I've seen them before. Sadly some of the tests are a bit flaky. Thanks for the patch!

@yorickpeterse
Copy link
Collaborator

These changes have been released in version 3.0.1.

RalfJung added a commit to RalfJung/miri that referenced this pull request Sep 2, 2022
…ion"

This reverts commit 5f3545e.
With tov/libffi-rs#58 landed, we no longer depend on abort_on_panic.
bors added a commit to rust-lang/miri that referenced this pull request Sep 2, 2022
re-enable FFI support

tov/libffi-rs#58 landed so the license should no longer be an issue. :)

Fixes #2526
@RalfJung RalfJung deleted the abort_on_panic branch September 2, 2022 12:41
@RalfJung
Copy link
Contributor Author

RalfJung commented Sep 2, 2022

Awesome, thanks. :)

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.

2 participants