{"action":"created","issue":{"url":"https://api.github.com/repos/go-interpreter/wagon/issues/59","repository_url":"https://api.github.com/repos/go-interpreter/wagon","labels_url":"https://api.github.com/repos/go-interpreter/wagon/issues/59/labels{/name}","comments_url":"https://api.github.com/repos/go-interpreter/wagon/issues/59/comments","events_url":"https://api.github.com/repos/go-interpreter/wagon/issues/59/events","html_url":"https://github.com/go-interpreter/wagon/pull/59","id":329136365,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkyNDY4MTcw","number":59,"title":"Pass the VM pointer to host go functions","user":{"login":"gballet","id":3272758,"node_id":"MDQ6VXNlcjMyNzI3NTg=","avatar_url":"https://avatars3.githubusercontent.com/u/3272758?v=4","gravatar_id":"","url":"https://api.github.com/users/gballet","html_url":"https://github.com/gballet","followers_url":"https://api.github.com/users/gballet/followers","following_url":"https://api.github.com/users/gballet/following{/other_user}","gists_url":"https://api.github.com/users/gballet/gists{/gist_id}","starred_url":"https://api.github.com/users/gballet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gballet/subscriptions","organizations_url":"https://api.github.com/users/gballet/orgs","repos_url":"https://api.github.com/users/gballet/repos","events_url":"https://api.github.com/users/gballet/events{/privacy}","received_events_url":"https://api.github.com/users/gballet/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-06-04T16:49:32Z","updated_at":"2018-06-06T06:49:34Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/go-interpreter/wagon/pulls/59","html_url":"https://github.com/go-interpreter/wagon/pull/59","diff_url":"https://github.com/go-interpreter/wagon/pull/59.diff","patch_url":"https://github.com/go-interpreter/wagon/pull/59.patch"},"body":"The previous PR #58 was enabling go functions, but without the VM context being passed, they can only do so much. This PR enforces that host Go functions are passed a `*VM` parameter as their first argument.\r\n\r\nAnother idea would be to check for pointers in the list of arguments, and pass a `*VM` each time that an argument is being encountered. It has some advantages, like streamlining the code and letting the Go type system handle the `panic`s for us. This being said, this leaves too much room for abuse since users could write confusing functions that take that pointer in the middle of their arguments list, thus disturbing the order to the WASM call.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/go-interpreter/wagon/59)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/go-interpreter/wagon/issues/comments/394959417","html_url":"https://github.com/go-interpreter/wagon/pull/59#issuecomment-394959417","issue_url":"https://api.github.com/repos/go-interpreter/wagon/issues/59","id":394959417,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NDk1OTQxNw==","user":{"login":"gballet","id":3272758,"node_id":"MDQ6VXNlcjMyNzI3NTg=","avatar_url":"https://avatars3.githubusercontent.com/u/3272758?v=4","gravatar_id":"","url":"https://api.github.com/users/gballet","html_url":"https://github.com/gballet","followers_url":"https://api.github.com/users/gballet/followers","following_url":"https://api.github.com/users/gballet/following{/other_user}","gists_url":"https://api.github.com/users/gballet/gists{/gist_id}","starred_url":"https://api.github.com/users/gballet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gballet/subscriptions","organizations_url":"https://api.github.com/users/gballet/orgs","repos_url":"https://api.github.com/users/gballet/repos","events_url":"https://api.github.com/users/gballet/events{/privacy}","received_events_url":"https://api.github.com/users/gballet/received_events","type":"User","site_admin":false},"created_at":"2018-06-06T06:49:34Z","updated_at":"2018-06-06T06:49:34Z","author_association":"CONTRIBUTOR","body":"@sbinet the motivation is in fact to let host functions that are passed a pointer, to access `memory`.\r\n\r\nAs far as I can tell, it is not possible to use closures to work around this problem. Consider the following code snippet:\r\n\r\n```go\r\n...\r\nm, err := wasm.ReadModule(bufio.NewReader(fd), ModuleResolver)\r\nif err != nil {\r\n\treturn errors.New(fmt.Sprintf(\"Could not read module: %v\", err))\r\n}\r\n\r\nvm, err := exec.NewVM(m)\r\nif err != nil {\r\n\treturn errors.New(fmt.Sprintf(\"Could not instantiate vm: %v\", err))\r\n}\r\n...\r\n```\r\n\r\n`ModuleResolver` does the function resolving, and therefore the \"linking\" to host functions. It is the one that should create the closure, but won't be able to capture `vm` because at the time it is called, `NewVM` hasn't been called yet.\r\n\r\nWhat I can propose is to \"inject\" `vm` each time I see a `ptr` in the list of parameters, this way the client code doesn't have to worry about getting a `*VM` object when they don't need it. What do you think?"}}
{"actor":{"display_login":"gballet"}}