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

[WIP] Get table from Ctx #245

Closed
wants to merge 7 commits into from
Closed

Conversation

tprk77
Copy link

@tprk77 tprk77 commented Mar 7, 2019

Based on some discussion on Spectrum, this change adds a table function to Ctx.

Feedback appreciated!

@lachlansneff
Copy link
Contributor

Looks great! Does it work for the specific issue you had?

@lachlansneff lachlansneff added the 📦 lib-deprecated About the deprecated crates label Mar 7, 2019
@tprk77
Copy link
Author

tprk77 commented Mar 7, 2019

I think there is a bit more to do here, but I'm still figuring it out. For example, Table has a set function, but I think now I need a get function. (Same thing for the AnyfuncTable.)

@lachlansneff
Copy link
Contributor

Yeah, I threw together the Table type pretty quickly without thinking about it too much. If you see a better way of doing it, we can definitely change it! :)

@tprk77 tprk77 changed the title Get table from Ctx WIP Get table from Ctx Mar 7, 2019
@tprk77 tprk77 changed the title WIP Get table from Ctx [WIP] Get table from Ctx Mar 7, 2019
@tprk77 tprk77 force-pushed the table-access branch 3 times, most recently from 2418bd6 to 4f5975d Compare March 23, 2019 17:15
@tprk77
Copy link
Author

tprk77 commented Mar 23, 2019

@lachlansneff I've had some time to look at this again, and I've been able to create and call a DynFunc from a vm::Ctx. But there's a problem. It appears that ProtectedCaller is not designed to work with non-export functions. I found this comment a little too late:

/// The functionality exposed by this trait is expected to be used
/// for calling functions exported by a webassembly module from
/// host code only.
pub trait ProtectedCaller: Send + Sync {

When I try to use DynFunc with a non-exported function, I get, predictably:

thread 'main' panicked at 'assertion failed: self.func_export_set.contains(&func_index)',
/.../wasmer/lib/clif-backend/src/signal/mod.rs:88:9

So I don't think this DynFunc approach is going to work, or anything else using ProtectedCaller::call. It seems like I actually need something like ProtectedCaller but a little more raw. I'm picturing something that takes a function pointer and a context directly. (Then does the trampoline, etc.)

Do you think that's a reasonable idea? Is that something I should try implementing?

@tprk77
Copy link
Author

tprk77 commented Aug 13, 2019

On second thought, this is all very out of date, and I don't think any of it is salvageable.

@tprk77 tprk77 closed this Aug 13, 2019
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.

3 participants