{"action":"opened","number":9,"pull_request":{"url":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9","id":89351284,"html_url":"https://github.com/dflemstr/v8-rs/pull/9","diff_url":"https://github.com/dflemstr/v8-rs/pull/9.diff","patch_url":"https://github.com/dflemstr/v8-rs/pull/9.patch","issue_url":"https://api.github.com/repos/dflemstr/v8-rs/issues/9","number":9,"state":"open","locked":false,"title":"Safe interface for storing rust values within JS objects","user":{"login":"tcosprojects","id":518742,"avatar_url":"https://avatars.githubusercontent.com/u/518742?v=3","gravatar_id":"","url":"https://api.github.com/users/tcosprojects","html_url":"https://github.com/tcosprojects","followers_url":"https://api.github.com/users/tcosprojects/followers","following_url":"https://api.github.com/users/tcosprojects/following{/other_user}","gists_url":"https://api.github.com/users/tcosprojects/gists{/gist_id}","starred_url":"https://api.github.com/users/tcosprojects/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tcosprojects/subscriptions","organizations_url":"https://api.github.com/users/tcosprojects/orgs","repos_url":"https://api.github.com/users/tcosprojects/repos","events_url":"https://api.github.com/users/tcosprojects/events{/privacy}","received_events_url":"https://api.github.com/users/tcosprojects/received_events","type":"User","site_admin":false},"body":"### Summary\r\nI've put together a safe* interface for holding rust objects within JS objects. I'm submitting this as a pull request to start a discussion over the design.\r\n\r\n### Design\r\nObjectTemplate, FunctionTemplate and FunctionCallbackInfo now accept a generic type T representing the type of the wrapped rust value. The PhantomData marker is used to carry the type information even though the pointer is stored within V8.\r\n\r\nThe default behavior (compatible with existing code) is to make this generic type the unit type. Objects that embed a rust object must be created through an ObjectTemplate<T> to ensure that that set_internal_field_count is called appropriately. Callbacks that use this type must be a FunctionTemplate<T> set on the ObjectTemplate<T> with the set_callback method. ObjectTemplate::set_internal_field_count is now marked unsafe because changing the value to 0 while using wrapped rust values would lead to undefined behavior.\r\n\r\nThe rust object is moved into the Object when creating an instance and can be considered owned by the V8 GC. Objects can be shared mutably with V8 by using an Rc<RefCell<T>>. A &mut reference is passed to the callback within the FunctionCallbackInfo struct.\r\n\r\n### Issues\r\n\r\n1. If the type of callback doesn't match the ObjectTemplate it fails to compile but with a fairly dense type error and references a line of code that doesn't clearly show the problem.\r\n\r\n```error[E0281]: type mismatch: the type `fn(v8::value::FunctionCallbackInfo<'_, std::rc::Rc<std::cell::RefCell<Foo>>>) -> v8::Value {test}` implements the trait `for<'r> std::ops::Fn<(v8::value::FunctionCallbackInfo<'r, std::rc::Rc<std::cell::RefCell<Foo>>>,)>`, but the trait `for<'r> std::ops::Fn<(v8::value::FunctionCallbackInfo<'r>,)>` is required (expected (), found struct `std::rc::Rc`)```\r\n\r\n\r\n2. A finalizer callback needs to drop the rust object when the JS object is garbage collected.\r\n3. ObjectTemplate might benefit from having two distinct types to represent the one with and without an internal rust object to prevent invalid combinations of method calls at compile time. It is safe, but may result in args.internal being None unexpectedly.\r\n4. The safety of this interface depends on certain combinations of ObjectTemplates and FunctionTemplates failing to compile. It would benefit from testing with compiletest-rs to ensure that these combinations result in a compile-time error.\r\n\r\n* Should be safe","created_at":"2016-10-14T09:25:09Z","updated_at":"2016-10-14T09:25:09Z","closed_at":null,"merged_at":null,"merge_commit_sha":"dc1f8a017fe4dc22995d9845449c26e66d24d434","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9/commits","review_comments_url":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9/comments","review_comment_url":"https://api.github.com/repos/dflemstr/v8-rs/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dflemstr/v8-rs/issues/9/comments","statuses_url":"https://api.github.com/repos/dflemstr/v8-rs/statuses/bbaac867a3d2a19cf34e7ffa21e5bbff688587d1","head":{"label":"tcosprojects:safe_internal","ref":"safe_internal","sha":"bbaac867a3d2a19cf34e7ffa21e5bbff688587d1","user":{"login":"tcosprojects","id":518742,"avatar_url":"https://avatars.githubusercontent.com/u/518742?v=3","gravatar_id":"","url":"https://api.github.com/users/tcosprojects","html_url":"https://github.com/tcosprojects","followers_url":"https://api.github.com/users/tcosprojects/followers","following_url":"https://api.github.com/users/tcosprojects/following{/other_user}","gists_url":"https://api.github.com/users/tcosprojects/gists{/gist_id}","starred_url":"https://api.github.com/users/tcosprojects/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tcosprojects/subscriptions","organizations_url":"https://api.github.com/users/tcosprojects/orgs","repos_url":"https://api.github.com/users/tcosprojects/repos","events_url":"https://api.github.com/users/tcosprojects/events{/privacy}","received_events_url":"https://api.github.com/users/tcosprojects/received_events","type":"User","site_admin":false},"repo":{"id":69788404,"name":"v8-rs","full_name":"tcosprojects/v8-rs","owner":{"login":"tcosprojects","id":518742,"avatar_url":"https://avatars.githubusercontent.com/u/518742?v=3","gravatar_id":"","url":"https://api.github.com/users/tcosprojects","html_url":"https://github.com/tcosprojects","followers_url":"https://api.github.com/users/tcosprojects/followers","following_url":"https://api.github.com/users/tcosprojects/following{/other_user}","gists_url":"https://api.github.com/users/tcosprojects/gists{/gist_id}","starred_url":"https://api.github.com/users/tcosprojects/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tcosprojects/subscriptions","organizations_url":"https://api.github.com/users/tcosprojects/orgs","repos_url":"https://api.github.com/users/tcosprojects/repos","events_url":"https://api.github.com/users/tcosprojects/events{/privacy}","received_events_url":"https://api.github.com/users/tcosprojects/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/tcosprojects/v8-rs","description":null,"fork":true,"url":"https://api.github.com/repos/tcosprojects/v8-rs","forks_url":"https://api.github.com/repos/tcosprojects/v8-rs/forks","keys_url":"https://api.github.com/repos/tcosprojects/v8-rs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/tcosprojects/v8-rs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/tcosprojects/v8-rs/teams","hooks_url":"https://api.github.com/repos/tcosprojects/v8-rs/hooks","issue_events_url":"https://api.github.com/repos/tcosprojects/v8-rs/issues/events{/number}","events_url":"https://api.github.com/repos/tcosprojects/v8-rs/events","assignees_url":"https://api.github.com/repos/tcosprojects/v8-rs/assignees{/user}","branches_url":"https://api.github.com/repos/tcosprojects/v8-rs/branches{/branch}","tags_url":"https://api.github.com/repos/tcosprojects/v8-rs/tags","blobs_url":"https://api.github.com/repos/tcosprojects/v8-rs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/tcosprojects/v8-rs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/tcosprojects/v8-rs/git/refs{/sha}","trees_url":"https://api.github.com/repos/tcosprojects/v8-rs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/tcosprojects/v8-rs/statuses/{sha}","languages_url":"https://api.github.com/repos/tcosprojects/v8-rs/languages","stargazers_url":"https://api.github.com/repos/tcosprojects/v8-rs/stargazers","contributors_url":"https://api.github.com/repos/tcosprojects/v8-rs/contributors","subscribers_url":"https://api.github.com/repos/tcosprojects/v8-rs/subscribers","subscription_url":"https://api.github.com/repos/tcosprojects/v8-rs/subscription","commits_url":"https://api.github.com/repos/tcosprojects/v8-rs/commits{/sha}","git_commits_url":"https://api.github.com/repos/tcosprojects/v8-rs/git/commits{/sha}","comments_url":"https://api.github.com/repos/tcosprojects/v8-rs/comments{/number}","issue_comment_url":"https://api.github.com/repos/tcosprojects/v8-rs/issues/comments{/number}","contents_url":"https://api.github.com/repos/tcosprojects/v8-rs/contents/{+path}","compare_url":"https://api.github.com/repos/tcosprojects/v8-rs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/tcosprojects/v8-rs/merges","archive_url":"https://api.github.com/repos/tcosprojects/v8-rs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/tcosprojects/v8-rs/downloads","issues_url":"https://api.github.com/repos/tcosprojects/v8-rs/issues{/number}","pulls_url":"https://api.github.com/repos/tcosprojects/v8-rs/pulls{/number}","milestones_url":"https://api.github.com/repos/tcosprojects/v8-rs/milestones{/number}","notifications_url":"https://api.github.com/repos/tcosprojects/v8-rs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/tcosprojects/v8-rs/labels{/name}","releases_url":"https://api.github.com/repos/tcosprojects/v8-rs/releases{/id}","deployments_url":"https://api.github.com/repos/tcosprojects/v8-rs/deployments","created_at":"2016-10-02T09:11:01Z","updated_at":"2016-10-02T09:11:04Z","pushed_at":"2016-10-14T09:24:59Z","git_url":"git://github.com/tcosprojects/v8-rs.git","ssh_url":"git@github.com:tcosprojects/v8-rs.git","clone_url":"https://github.com/tcosprojects/v8-rs.git","svn_url":"https://github.com/tcosprojects/v8-rs","homepage":null,"size":2946,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"dflemstr:master","ref":"master","sha":"74d5a4e8019e9ef2cd42ee8798484006d52fb79a","user":{"login":"dflemstr","id":56029,"avatar_url":"https://avatars.githubusercontent.com/u/56029?v=3","gravatar_id":"","url":"https://api.github.com/users/dflemstr","html_url":"https://github.com/dflemstr","followers_url":"https://api.github.com/users/dflemstr/followers","following_url":"https://api.github.com/users/dflemstr/following{/other_user}","gists_url":"https://api.github.com/users/dflemstr/gists{/gist_id}","starred_url":"https://api.github.com/users/dflemstr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dflemstr/subscriptions","organizations_url":"https://api.github.com/users/dflemstr/orgs","repos_url":"https://api.github.com/users/dflemstr/repos","events_url":"https://api.github.com/users/dflemstr/events{/privacy}","received_events_url":"https://api.github.com/users/dflemstr/received_events","type":"User","site_admin":false},"repo":{"id":67560486,"name":"v8-rs","full_name":"dflemstr/v8-rs","owner":{"login":"dflemstr","id":56029,"avatar_url":"https://avatars.githubusercontent.com/u/56029?v=3","gravatar_id":"","url":"https://api.github.com/users/dflemstr","html_url":"https://github.com/dflemstr","followers_url":"https://api.github.com/users/dflemstr/followers","following_url":"https://api.github.com/users/dflemstr/following{/other_user}","gists_url":"https://api.github.com/users/dflemstr/gists{/gist_id}","starred_url":"https://api.github.com/users/dflemstr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dflemstr/subscriptions","organizations_url":"https://api.github.com/users/dflemstr/orgs","repos_url":"https://api.github.com/users/dflemstr/repos","events_url":"https://api.github.com/users/dflemstr/events{/privacy}","received_events_url":"https://api.github.com/users/dflemstr/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/dflemstr/v8-rs","description":null,"fork":false,"url":"https://api.github.com/repos/dflemstr/v8-rs","forks_url":"https://api.github.com/repos/dflemstr/v8-rs/forks","keys_url":"https://api.github.com/repos/dflemstr/v8-rs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dflemstr/v8-rs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dflemstr/v8-rs/teams","hooks_url":"https://api.github.com/repos/dflemstr/v8-rs/hooks","issue_events_url":"https://api.github.com/repos/dflemstr/v8-rs/issues/events{/number}","events_url":"https://api.github.com/repos/dflemstr/v8-rs/events","assignees_url":"https://api.github.com/repos/dflemstr/v8-rs/assignees{/user}","branches_url":"https://api.github.com/repos/dflemstr/v8-rs/branches{/branch}","tags_url":"https://api.github.com/repos/dflemstr/v8-rs/tags","blobs_url":"https://api.github.com/repos/dflemstr/v8-rs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dflemstr/v8-rs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dflemstr/v8-rs/git/refs{/sha}","trees_url":"https://api.github.com/repos/dflemstr/v8-rs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dflemstr/v8-rs/statuses/{sha}","languages_url":"https://api.github.com/repos/dflemstr/v8-rs/languages","stargazers_url":"https://api.github.com/repos/dflemstr/v8-rs/stargazers","contributors_url":"https://api.github.com/repos/dflemstr/v8-rs/contributors","subscribers_url":"https://api.github.com/repos/dflemstr/v8-rs/subscribers","subscription_url":"https://api.github.com/repos/dflemstr/v8-rs/subscription","commits_url":"https://api.github.com/repos/dflemstr/v8-rs/commits{/sha}","git_commits_url":"https://api.github.com/repos/dflemstr/v8-rs/git/commits{/sha}","comments_url":"https://api.github.com/repos/dflemstr/v8-rs/comments{/number}","issue_comment_url":"https://api.github.com/repos/dflemstr/v8-rs/issues/comments{/number}","contents_url":"https://api.github.com/repos/dflemstr/v8-rs/contents/{+path}","compare_url":"https://api.github.com/repos/dflemstr/v8-rs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dflemstr/v8-rs/merges","archive_url":"https://api.github.com/repos/dflemstr/v8-rs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dflemstr/v8-rs/downloads","issues_url":"https://api.github.com/repos/dflemstr/v8-rs/issues{/number}","pulls_url":"https://api.github.com/repos/dflemstr/v8-rs/pulls{/number}","milestones_url":"https://api.github.com/repos/dflemstr/v8-rs/milestones{/number}","notifications_url":"https://api.github.com/repos/dflemstr/v8-rs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dflemstr/v8-rs/labels{/name}","releases_url":"https://api.github.com/repos/dflemstr/v8-rs/releases{/id}","deployments_url":"https://api.github.com/repos/dflemstr/v8-rs/deployments","created_at":"2016-09-07T01:27:51Z","updated_at":"2016-10-12T18:22:13Z","pushed_at":"2016-10-14T09:25:09Z","git_url":"git://github.com/dflemstr/v8-rs.git","ssh_url":"git@github.com:dflemstr/v8-rs.git","clone_url":"https://github.com/dflemstr/v8-rs.git","svn_url":"https://github.com/dflemstr/v8-rs","homepage":null,"size":3021,"stargazers_count":8,"watchers_count":8,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1,"mirror_url":null,"open_issues_count":1,"forks":1,"open_issues":1,"watchers":8,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9"},"html":{"href":"https://github.com/dflemstr/v8-rs/pull/9"},"issue":{"href":"https://api.github.com/repos/dflemstr/v8-rs/issues/9"},"comments":{"href":"https://api.github.com/repos/dflemstr/v8-rs/issues/9/comments"},"review_comments":{"href":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9/comments"},"review_comment":{"href":"https://api.github.com/repos/dflemstr/v8-rs/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dflemstr/v8-rs/pulls/9/commits"},"statuses":{"href":"https://api.github.com/repos/dflemstr/v8-rs/statuses/bbaac867a3d2a19cf34e7ffa21e5bbff688587d1"}},"merged":false,"mergeable":true,"mergeable_state":"clean","merged_by":null,"comments":0,"review_comments":0,"commits":2,"additions":175,"deletions":35,"changed_files":4}}
{"actor":{"display_login":"tcosprojects"}}