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

Add option to extract opaque bodies #121

Merged
merged 10 commits into from
Apr 17, 2024
Merged

Add option to extract opaque bodies #121

merged 10 commits into from
Apr 17, 2024

Conversation

Nadrieril
Copy link
Member

@Nadrieril Nadrieril commented Apr 12, 2024

With --extract-opaque-bodies, we now also extract foreign globals, methods, trait provided methods, and ADT definitions. This is limited by two things:

  • not all function bodies have a MIR in the crate data (only inlineable and generic functions do);
  • when function have MIR available it's only optimized MIR, which we don't support as well.

This PR includes a few tests that fail because of other problems. Two of them are hax limitations that I'm ignoring until we're back on hax master. I opened issues for the other two: #123, #120.

The PR also includes many improvements to the ui tests that I needed along the way.

Fixes #114

@Nadrieril Nadrieril force-pushed the extract-nonlocal-bodies branch from cca882d to 8d828fb Compare April 12, 2024 14:31
@Nadrieril Nadrieril force-pushed the extract-nonlocal-bodies branch from 8d828fb to 1d9b7a0 Compare April 12, 2024 17:43
@Nadrieril Nadrieril marked this pull request as ready for review April 12, 2024 17:43
@Nadrieril Nadrieril force-pushed the extract-nonlocal-bodies branch from 1d9b7a0 to 8788fa7 Compare April 12, 2024 17:49
@Nadrieril Nadrieril force-pushed the extract-nonlocal-bodies branch from 8788fa7 to 8bc972c Compare April 12, 2024 17:52
Copy link
Member

@sonmarcho sonmarcho left a comment

Choose a reason for hiding this comment

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

The fact that we can't always retrieve the bodies of the external functions makes the effects of this PR quite limited, unfortunately. I wonder how the Kani people do, as I thought Kani looked into the bodies of the external definitions (@zhassan-aws could you comment on that?).
This being said, all the improvements you added to the UI tests are good and most welcome.

charon/src/translate_functions_to_ullbc.rs Show resolved Hide resolved
charon/src/translate_traits.rs Outdated Show resolved Hide resolved
charon/src/translate_functions_to_ullbc.rs Outdated Show resolved Hide resolved
@Nadrieril
Copy link
Member Author

Nadrieril commented Apr 16, 2024

We could pass -Zalways-encode-mir (or -Zcross-crate-inline-threshold=yes?) to dependencies, I think this ensures we get the MIR for everything (while we're at it, -Zno-codegen avoids generating code we don't need).

This doesn't work for std though; for that, this proposal might be the solution, as it proposes shipping a std with full MIR: rust-lang/compiler-team#738. Or we could ship our own MIR-only sysroot.

@zhassan-aws
Copy link
Contributor

The fact that we can't always retrieve the bodies of the external functions makes the effects of this PR quite limited, unfortunately. I wonder how the Kani people do, as I thought Kani looked into the bodies of the external definitions (@zhassan-aws could you comment on that?).

In Kani, we run the kani-compiler on each of the dependent crates to generate the MIR only. Then, when compiling the target user crate, the kani-compiler has access to the MIR of all the dependent crates. This also applies to std, but it's handled in a special way.

See this RFC and one of the PRs that implements it for more information.

Feel free to reach out for further information.

@Nadrieril
Copy link
Member Author

Thank you for these links!

@sonmarcho
Copy link
Member

This is useful documentation, thanks!

Copy link
Member

@sonmarcho sonmarcho left a comment

Choose a reason for hiding this comment

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

Looks good: it indeed makes sense to do check once and for all in translate_body whether we should extract the body or not.

@Nadrieril Nadrieril merged commit 8a19ce6 into main Apr 17, 2024
2 checks passed
@Nadrieril Nadrieril deleted the extract-nonlocal-bodies branch April 17, 2024 11:02
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.

Add an option to extract the bodies of the external definitions
3 participants