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.

Sign up for GitHub

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

Jump to bottom

feat(vm) Add Instance::offsets and InstanceHandle::vmoffsets #2040

Merged
merged 4 commits into from
Jan 26, 2021
Merged

feat(vm) Add Instance::offsets and InstanceHandle::vmoffsets #2040

merged 4 commits into from
Jan 26, 2021

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Jan 22, 2021 •
edited
Loading

Description

This PR adds Instance::offets and InstanceHandle::vmoffsets. It fixes #2032.
This PR also cleans up the mod.rs module by extracting the InstanceRef implementation into its own ref.rs module.

Review

  • Add a short description of the the change to the CHANGELOG.md file

@Hywan Hywan added 🎉 enhancement New feature! 📦 lib-vm About wasmer-vm labels Jan 22, 2021
@Hywan Hywan self-assigned this Jan 22, 2021
@Hywan
Copy link
Contributor Author

Hywan commented Jan 22, 2021

bors try

bors bot added a commit that referenced this pull request Jan 22, 2021
@bors
Try #2040:
c065cfe
@Hywan Hywan mentioned this pull request Jan 22, 2021
Closed
jubianchi
jubianchi approved these changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@jubianchi jubianchi left a comment

Choose a reason for hiding this comment

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

Seems good, waiting for @Geal feedback on that.

@bors
Copy link
Contributor

bors bot commented Jan 22, 2021

try

Build failed:

MarkMcCaskey
MarkMcCaskey approved these changes Jan 22, 2021
View reviewed changes
lib/vm/src/instance/mod.rs
Comment on lines +944 to +946
/// Return a reference to the `VMOffsets` to get offsets in the
/// `Self::vmctx_ptr` region. Be careful when doing pointer
/// arithmetic!
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we need to document anything here or if this changes anything in regards to what is public / what we're saying we won't break, but something to keep in mind here!

@Hywan
Copy link
Contributor Author

Hywan commented Jan 26, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 26, 2021

Build succeeded:

@bors bors bot merged commit fa0938c into wasmerio:master Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jubianchi jubianchi jubianchi approved these changes

@MarkMcCaskey MarkMcCaskey MarkMcCaskey approved these changes

@nlewycky nlewycky Awaiting requested review from nlewycky

@syrusakbary syrusakbary Awaiting requested review from syrusakbary

Assignees

@Hywan Hywan

Labels
🎉 enhancement New feature! 📦 lib-vm About wasmer-vm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Making VMOffsets accessible from the InstanceHandle
3 participants
@Hywan @jubianchi @MarkMcCaskey