{"action":"created","comment":{"url":"https://api.github.com/repos/facebook/prepack/pulls/comments/169742931","pull_request_review_id":98319043,"id":169742931,"diff_hunk":"@@ -211,21 +211,25 @@ function ensureIsNotFinal(realm: Realm, O: ObjectValue, P: void | PropertyKeyVal\n   if (!O.isFinalObject()) {\n     return;\n   }\n-  if (!realm.isInPureScope()) {\n-    // We can't continue because this object is already in its final state.\n-    AbstractValue.reportIntrospectionError(O, P);\n-    throw new FatalError();\n-  }\n-  // It's not safe to write to this object anymore because it's already\n-  // been used in a way that serializes its final state. We can, however,\n-  // leak it if we're in pure scope, and continue to emit assignments.\n-  Leak.leakValue(realm, O);\n-  if (O.isLeakedObject()) {\n-    return;\n+  if (realm.isInPureScope()) {\n+    // It's not safe to write to this object anymore because it's already\n+    // been used in a way that serializes its final state. We can, however,\n+    // leak it if we're in pure scope, and continue to emit assignments.\n+    Leak.leakValue(realm, O);\n+    if (O.isLeakedObject()) {\n+      return;\n+    }\n   }\n-  // The object was created outside of pure scope so we couldn't leak.\n-  // Give up.\n-  AbstractValue.reportIntrospectionError(O, P);\n+  // We can't continue because this object is already in its final state.\n+  let error = new CompilerDiagnostic(\n+    \"Mutating an object that has been marked as final is not yet supported \" +","path":"src/methods/properties.js","position":null,"original_position":38,"commit_id":"7041527c1c2cf3ad5d01dcea333513083382b9ad","original_commit_id":"2b765e82c3384401aeb1a8e3066f51f4bf2692bb","user":{"login":"gaearon","id":810438,"avatar_url":"https://avatars0.githubusercontent.com/u/810438?v=4","gravatar_id":"","url":"https://api.github.com/users/gaearon","html_url":"https://github.com/gaearon","followers_url":"https://api.github.com/users/gaearon/followers","following_url":"https://api.github.com/users/gaearon/following{/other_user}","gists_url":"https://api.github.com/users/gaearon/gists{/gist_id}","starred_url":"https://api.github.com/users/gaearon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gaearon/subscriptions","organizations_url":"https://api.github.com/users/gaearon/orgs","repos_url":"https://api.github.com/users/gaearon/repos","events_url":"https://api.github.com/users/gaearon/events{/privacy}","received_events_url":"https://api.github.com/users/gaearon/received_events","type":"User","site_admin":false},"body":"Addressed","created_at":"2018-02-21T18:59:41Z","updated_at":"2018-02-21T18:59:42Z","html_url":"https://github.com/facebook/prepack/pull/1469#discussion_r169742931","pull_request_url":"https://api.github.com/repos/facebook/prepack/pulls/1469","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/facebook/prepack/pulls/comments/169742931"},"html":{"href":"https://github.com/facebook/prepack/pull/1469#discussion_r169742931"},"pull_request":{"href":"https://api.github.com/repos/facebook/prepack/pulls/1469"}},"in_reply_to_id":169742008},"pull_request":{"url":"https://api.github.com/repos/facebook/prepack/pulls/1469","id":169808688,"html_url":"https://github.com/facebook/prepack/pull/1469","diff_url":"https://github.com/facebook/prepack/pull/1469.diff","patch_url":"https://github.com/facebook/prepack/pull/1469.patch","issue_url":"https://api.github.com/repos/facebook/prepack/issues/1469","number":1469,"state":"open","locked":false,"title":"Make Object.assign() safe to use with simple partial sources","user":{"login":"gaearon","id":810438,"avatar_url":"https://avatars0.githubusercontent.com/u/810438?v=4","gravatar_id":"","url":"https://api.github.com/users/gaearon","html_url":"https://github.com/gaearon","followers_url":"https://api.github.com/users/gaearon/followers","following_url":"https://api.github.com/users/gaearon/following{/other_user}","gists_url":"https://api.github.com/users/gaearon/gists{/gist_id}","starred_url":"https://api.github.com/users/gaearon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gaearon/subscriptions","organizations_url":"https://api.github.com/users/gaearon/orgs","repos_url":"https://api.github.com/users/gaearon/repos","events_url":"https://api.github.com/users/gaearon/events{/privacy}","received_events_url":"https://api.github.com/users/gaearon/received_events","type":"User","site_admin":false},"body":"Fixes https://github.com/facebook/prepack/issues/1462.\r\n\r\nSupersedes my attempts in https://github.com/facebook/prepack/pull/1459, https://github.com/facebook/prepack/pull/1460, and https://github.com/facebook/prepack/pull/1468.\r\nThe git history reflects a lot of back-and-forth so it might make more sense to look at the final code.\r\n\r\nThere are two separate fixes here:\r\n\r\n* If we have a partial simple source and then a normal source, we shouldn't attempt to `Set` any further keys after the first partial. Instead we should emit residual `Object.assign` calls. If we set the keys, we end up with a wrong key order (a key gets set earlier than it should have). Key order (AFAIK) isn't mandated by the spec but tends to be relied on in product code because all engines preserve it in the order of assignments.\r\n\r\n* It is not safe to mutate either a partial source or the target (when some sources are partial) in Prepack land after the `Object.assign` call. This is because Prepack will serialize all those mutations (as the object final state) right into the residual `Object.assign` call. But this is wrong because they haven't happened yet.\r\n\r\nThe second issue is thorny. In many cases sources (and the target) don't get mutated later. So we'd like to keep supporting `Object.assign` with partials when we don't touch them later.\r\n\r\nTo solve it, I introduced new `obj.makeFinal()` and `obj.isFinalObject()` methods. If an object is marked as \"final\", we cannot make changes to it that would change its emitted form. This is a way for us to say \"let's try to continue as far as we can, but if anything tries to touch it, fatal\".\r\n\r\nIt doesn't mean the object is \"frozen\" (technically we didn't freeze it), but it means Prepack can't deal with further mutations to it because Prepack has already \"used\" its current snapshot and it would be unsafe to change it.\r\n\r\nI'm not sure I'm checking `isFinalObject()` in all places where it's necessary. If there is a way to do this with stronger guarantees please let me know. I want to still highlight that this fixes multiple existing issues on master so it's a step in a safer duration, even though I'm wary that if the concept is not fully baked, it might give a false sense of security.\r\n\r\nHere's how we're using this new infra. When we emit a residual `Object.assign()` call because some sources are partial (and simple), I mark all sources and the target as \"final\". So if they don't get mutated, everything is fine. If they get mutated later, we get a fatal when they do. In the future Prepack might support this better, but the fatal makes it clear which cases it currently doesn't handle (and would produce wrong output for).\r\n\r\nThere might also be other cases where making an object as \"final\" lets us continue further, although I don't know Prepack codebase well enough to tell for sure. So I'm hoping it is useful as a generic concept outside of the `Object.assign()` use case.\r\n\r\nThe pure mode is treated a bit differently. In the pure mode we can get away with \"leaking\" final values whenever we need to write to them. Then they're treated like any other \"leaked\" object.\r\n\r\nI added regression tests for all existing cases I could find, both in pure and impure mode.","created_at":"2018-02-18T00:32:52Z","updated_at":"2018-02-21T18:59:42Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3508a66342c99e5599e1c85132f9b67b608d33e1","assignee":null,"assignees":[],"requested_reviewers":[{"login":"sebmarkbage","id":63648,"avatar_url":"https://avatars2.githubusercontent.com/u/63648?v=4","gravatar_id":"","url":"https://api.github.com/users/sebmarkbage","html_url":"https://github.com/sebmarkbage","followers_url":"https://api.github.com/users/sebmarkbage/followers","following_url":"https://api.github.com/users/sebmarkbage/following{/other_user}","gists_url":"https://api.github.com/users/sebmarkbage/gists{/gist_id}","starred_url":"https://api.github.com/users/sebmarkbage/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sebmarkbage/subscriptions","organizations_url":"https://api.github.com/users/sebmarkbage/orgs","repos_url":"https://api.github.com/users/sebmarkbage/repos","events_url":"https://api.github.com/users/sebmarkbage/events{/privacy}","received_events_url":"https://api.github.com/users/sebmarkbage/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[{"id":279345275,"url":"https://api.github.com/repos/facebook/prepack/labels/CLA%20Signed","name":"CLA Signed","color":"009900","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/facebook/prepack/pulls/1469/commits","review_comments_url":"https://api.github.com/repos/facebook/prepack/pulls/1469/comments","review_comment_url":"https://api.github.com/repos/facebook/prepack/pulls/comments{/number}","comments_url":"https://api.github.com/repos/facebook/prepack/issues/1469/comments","statuses_url":"https://api.github.com/repos/facebook/prepack/statuses/7041527c1c2cf3ad5d01dcea333513083382b9ad","head":{"label":"gaearon:assign-issue-continued-1.555","ref":"assign-issue-continued-1.555","sha":"7041527c1c2cf3ad5d01dcea333513083382b9ad","user":{"login":"gaearon","id":810438,"avatar_url":"https://avatars0.githubusercontent.com/u/810438?v=4","gravatar_id":"","url":"https://api.github.com/users/gaearon","html_url":"https://github.com/gaearon","followers_url":"https://api.github.com/users/gaearon/followers","following_url":"https://api.github.com/users/gaearon/following{/other_user}","gists_url":"https://api.github.com/users/gaearon/gists{/gist_id}","starred_url":"https://api.github.com/users/gaearon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gaearon/subscriptions","organizations_url":"https://api.github.com/users/gaearon/orgs","repos_url":"https://api.github.com/users/gaearon/repos","events_url":"https://api.github.com/users/gaearon/events{/privacy}","received_events_url":"https://api.github.com/users/gaearon/received_events","type":"User","site_admin":false},"repo":{"id":118929792,"name":"prepack","full_name":"gaearon/prepack","owner":{"login":"gaearon","id":810438,"avatar_url":"https://avatars0.githubusercontent.com/u/810438?v=4","gravatar_id":"","url":"https://api.github.com/users/gaearon","html_url":"https://github.com/gaearon","followers_url":"https://api.github.com/users/gaearon/followers","following_url":"https://api.github.com/users/gaearon/following{/other_user}","gists_url":"https://api.github.com/users/gaearon/gists{/gist_id}","starred_url":"https://api.github.com/users/gaearon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gaearon/subscriptions","organizations_url":"https://api.github.com/users/gaearon/orgs","repos_url":"https://api.github.com/users/gaearon/repos","events_url":"https://api.github.com/users/gaearon/events{/privacy}","received_events_url":"https://api.github.com/users/gaearon/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/gaearon/prepack","description":"Prepack is a partial evaluator for JavaScript. Prepack rewrites a JavaScript bundle, resulting in JavaScript code that executes more efficiently.","fork":true,"url":"https://api.github.com/repos/gaearon/prepack","forks_url":"https://api.github.com/repos/gaearon/prepack/forks","keys_url":"https://api.github.com/repos/gaearon/prepack/keys{/key_id}","collaborators_url":"https://api.github.com/repos/gaearon/prepack/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/gaearon/prepack/teams","hooks_url":"https://api.github.com/repos/gaearon/prepack/hooks","issue_events_url":"https://api.github.com/repos/gaearon/prepack/issues/events{/number}","events_url":"https://api.github.com/repos/gaearon/prepack/events","assignees_url":"https://api.github.com/repos/gaearon/prepack/assignees{/user}","branches_url":"https://api.github.com/repos/gaearon/prepack/branches{/branch}","tags_url":"https://api.github.com/repos/gaearon/prepack/tags","blobs_url":"https://api.github.com/repos/gaearon/prepack/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/gaearon/prepack/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/gaearon/prepack/git/refs{/sha}","trees_url":"https://api.github.com/repos/gaearon/prepack/git/trees{/sha}","statuses_url":"https://api.github.com/repos/gaearon/prepack/statuses/{sha}","languages_url":"https://api.github.com/repos/gaearon/prepack/languages","stargazers_url":"https://api.github.com/repos/gaearon/prepack/stargazers","contributors_url":"https://api.github.com/repos/gaearon/prepack/contributors","subscribers_url":"https://api.github.com/repos/gaearon/prepack/subscribers","subscription_url":"https://api.github.com/repos/gaearon/prepack/subscription","commits_url":"https://api.github.com/repos/gaearon/prepack/commits{/sha}","git_commits_url":"https://api.github.com/repos/gaearon/prepack/git/commits{/sha}","comments_url":"https://api.github.com/repos/gaearon/prepack/comments{/number}","issue_comment_url":"https://api.github.com/repos/gaearon/prepack/issues/comments{/number}","contents_url":"https://api.github.com/repos/gaearon/prepack/contents/{+path}","compare_url":"https://api.github.com/repos/gaearon/prepack/compare/{base}...{head}","merges_url":"https://api.github.com/repos/gaearon/prepack/merges","archive_url":"https://api.github.com/repos/gaearon/prepack/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/gaearon/prepack/downloads","issues_url":"https://api.github.com/repos/gaearon/prepack/issues{/number}","pulls_url":"https://api.github.com/repos/gaearon/prepack/pulls{/number}","milestones_url":"https://api.github.com/repos/gaearon/prepack/milestones{/number}","notifications_url":"https://api.github.com/repos/gaearon/prepack/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/gaearon/prepack/labels{/name}","releases_url":"https://api.github.com/repos/gaearon/prepack/releases{/id}","deployments_url":"https://api.github.com/repos/gaearon/prepack/deployments","created_at":"2018-01-25T15:21:25Z","updated_at":"2018-01-29T05:11:27Z","pushed_at":"2018-02-21T18:59:33Z","git_url":"git://github.com/gaearon/prepack.git","ssh_url":"git@github.com:gaearon/prepack.git","clone_url":"https://github.com/gaearon/prepack.git","svn_url":"https://github.com/gaearon/prepack","homepage":"https://prepack.io","size":6655,"stargazers_count":1,"watchers_count":1,"language":"JavaScript","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"facebook:master","ref":"master","sha":"bfc005a15bfacf429708a48aa2eb84bfa9c97163","user":{"login":"facebook","id":69631,"avatar_url":"https://avatars3.githubusercontent.com/u/69631?v=4","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"repo":{"id":45147841,"name":"prepack","full_name":"facebook/prepack","owner":{"login":"facebook","id":69631,"avatar_url":"https://avatars3.githubusercontent.com/u/69631?v=4","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/facebook/prepack","description":"Prepack is a partial evaluator for JavaScript. Prepack rewrites a JavaScript bundle, resulting in JavaScript code that executes more efficiently.","fork":false,"url":"https://api.github.com/repos/facebook/prepack","forks_url":"https://api.github.com/repos/facebook/prepack/forks","keys_url":"https://api.github.com/repos/facebook/prepack/keys{/key_id}","collaborators_url":"https://api.github.com/repos/facebook/prepack/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/facebook/prepack/teams","hooks_url":"https://api.github.com/repos/facebook/prepack/hooks","issue_events_url":"https://api.github.com/repos/facebook/prepack/issues/events{/number}","events_url":"https://api.github.com/repos/facebook/prepack/events","assignees_url":"https://api.github.com/repos/facebook/prepack/assignees{/user}","branches_url":"https://api.github.com/repos/facebook/prepack/branches{/branch}","tags_url":"https://api.github.com/repos/facebook/prepack/tags","blobs_url":"https://api.github.com/repos/facebook/prepack/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/facebook/prepack/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/facebook/prepack/git/refs{/sha}","trees_url":"https://api.github.com/repos/facebook/prepack/git/trees{/sha}","statuses_url":"https://api.github.com/repos/facebook/prepack/statuses/{sha}","languages_url":"https://api.github.com/repos/facebook/prepack/languages","stargazers_url":"https://api.github.com/repos/facebook/prepack/stargazers","contributors_url":"https://api.github.com/repos/facebook/prepack/contributors","subscribers_url":"https://api.github.com/repos/facebook/prepack/subscribers","subscription_url":"https://api.github.com/repos/facebook/prepack/subscription","commits_url":"https://api.github.com/repos/facebook/prepack/commits{/sha}","git_commits_url":"https://api.github.com/repos/facebook/prepack/git/commits{/sha}","comments_url":"https://api.github.com/repos/facebook/prepack/comments{/number}","issue_comment_url":"https://api.github.com/repos/facebook/prepack/issues/comments{/number}","contents_url":"https://api.github.com/repos/facebook/prepack/contents/{+path}","compare_url":"https://api.github.com/repos/facebook/prepack/compare/{base}...{head}","merges_url":"https://api.github.com/repos/facebook/prepack/merges","archive_url":"https://api.github.com/repos/facebook/prepack/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/facebook/prepack/downloads","issues_url":"https://api.github.com/repos/facebook/prepack/issues{/number}","pulls_url":"https://api.github.com/repos/facebook/prepack/pulls{/number}","milestones_url":"https://api.github.com/repos/facebook/prepack/milestones{/number}","notifications_url":"https://api.github.com/repos/facebook/prepack/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/facebook/prepack/labels{/name}","releases_url":"https://api.github.com/repos/facebook/prepack/releases{/id}","deployments_url":"https://api.github.com/repos/facebook/prepack/deployments","created_at":"2015-10-28T23:25:08Z","updated_at":"2018-02-21T18:34:49Z","pushed_at":"2018-02-21T18:59:35Z","git_url":"git://github.com/facebook/prepack.git","ssh_url":"git@github.com:facebook/prepack.git","clone_url":"https://github.com/facebook/prepack.git","svn_url":"https://github.com/facebook/prepack","homepage":"https://prepack.io","size":6858,"stargazers_count":11205,"watchers_count":11205,"language":"JavaScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":333,"mirror_url":null,"archived":false,"open_issues_count":151,"license":null,"forks":333,"open_issues":151,"watchers":11205,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/facebook/prepack/pulls/1469"},"html":{"href":"https://github.com/facebook/prepack/pull/1469"},"issue":{"href":"https://api.github.com/repos/facebook/prepack/issues/1469"},"comments":{"href":"https://api.github.com/repos/facebook/prepack/issues/1469/comments"},"review_comments":{"href":"https://api.github.com/repos/facebook/prepack/pulls/1469/comments"},"review_comment":{"href":"https://api.github.com/repos/facebook/prepack/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/facebook/prepack/pulls/1469/commits"},"statuses":{"href":"https://api.github.com/repos/facebook/prepack/statuses/7041527c1c2cf3ad5d01dcea333513083382b9ad"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"gaearon"}}