From 5f626ebdbfae519142f2b34f581ef3bfa72a10a8 Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <dg@domgan.com>
Date: Tue, 17 Jul 2018 06:13:07 -0700
Subject: [PATCH] Ensure Object.assign merge optimization properly accounts for
 prefix args (#2261)

Summary:
Release notes: none

When we check to do the temporal `Object.assign` merge optimization, we should properly account for any "prefix" args that occur before "to" and the "suffix" args. Test case added that came up in testing.
Pull Request resolved: https://github.com/facebook/prepack/pull/2261

Differential Revision: D8874714

Pulled By: trueadm

fbshipit-source-id: 59869f556c1300424c5a1b59d3df1fdd139b41fe
---
 src/utils/generator.js                        | 15 ++++------
 .../optimized-functions/DeadObjectAssign23.js | 28 +++++++++++++++++++
 .../optimized-functions/DeadObjectAssign24.js | 28 +++++++++++++++++++
 3 files changed, 62 insertions(+), 9 deletions(-)
 create mode 100644 test/serializer/optimized-functions/DeadObjectAssign23.js
 create mode 100644 test/serializer/optimized-functions/DeadObjectAssign24.js

diff --git a/src/utils/generator.js b/src/utils/generator.js
index 1aa093156..8c6635ef1 100644
--- a/src/utils/generator.js
+++ b/src/utils/generator.js
@@ -1361,7 +1361,7 @@ export function attemptToMergeEquivalentObjectAssigns(
       }
       let otherArgs = otherTemporalBuildNodeEntry.args;
       // Object.assign has at least 1 arg
-      if (otherArgs.length < 2) {
+      if (otherArgs.length < 1) {
         continue;
       }
       let otherArgsToUse = [];
@@ -1404,15 +1404,12 @@ export function attemptToMergeEquivalentObjectAssigns(
       // If we cannot omit the "to" value that means it's being used, so we shall not try to
       // optimize this Object.assign.
       if (!callbacks.canOmit(to)) {
-        let newArgs = [to, ...otherArgsToUse];
+        // our merged Object.assign, shoud look like:
+        // Object.assign(to, ...prefixArgs, ...otherArgsToUse, ...suffixArgs)
+        let prefixArgs = args.slice(1, i - 1); // We start at 1, as 0 is the index of "to" a
+        let suffixArgs = args.slice(i + 1);
+        let newArgs = [to, ...prefixArgs, ...otherArgsToUse, ...suffixArgs];
 
-        for (let x = 2; x < args.length; x++) {
-          let arg = args[x];
-          // We don't want to add the "to" that we're merging with!
-          if (arg !== possibleOtherObjectAssignTo) {
-            newArgs.push(arg);
-          }
-        }
         // We now create a new TemporalObjectAssignEntry, without mutating the existing
         // entry at this point. This new entry is essentially a TemporalObjectAssignEntry
         // that contains two Object.assign call TemporalObjectAssignEntry entries that have
diff --git a/test/serializer/optimized-functions/DeadObjectAssign23.js b/test/serializer/optimized-functions/DeadObjectAssign23.js
new file mode 100644
index 000000000..db889ff25
--- /dev/null
+++ b/test/serializer/optimized-functions/DeadObjectAssign23.js
@@ -0,0 +1,28 @@
+// Copies of _\$C\(:1
+// Copies of var _\$C = _\$B.assign;:1
+// inline expressions
+
+// _$C is the variable for Object.assign. See DeadObjectAssign4.js for
+// a larger explanation.
+
+function fn(obj, x) {
+  var a = Object.assign({}, x, { a: 1 });
+  var b = Object.assign({}, x, { b: 1 });
+
+  if (obj.cond) {
+    var c = Object.assign({}, a, b);
+
+    return c.a + c.b;
+  }
+}
+
+this.__optimize && __optimize(fn);
+
+inspect = function() {
+  return fn(
+    {
+      cond: true,
+    },
+    { a: 0, b: 0 }
+  );
+};
diff --git a/test/serializer/optimized-functions/DeadObjectAssign24.js b/test/serializer/optimized-functions/DeadObjectAssign24.js
new file mode 100644
index 000000000..990ca91d3
--- /dev/null
+++ b/test/serializer/optimized-functions/DeadObjectAssign24.js
@@ -0,0 +1,28 @@
+// Copies of _\$D\(:1
+// Copies of var _\$D = _\$C.assign;:1
+// inline expressions
+
+// _$C is the variable for Object.assign. See DeadObjectAssign4.js for
+// a larger explanation.
+
+function fn(obj, x) {
+  var a = Object.assign({}, x, { a: 1 });
+  var b = Object.assign({}, x, { b: 1 });
+
+  if (obj.cond) {
+    var c = Object.assign({}, a, b, a);
+
+    return c.a + c.b;
+  }
+}
+
+this.__optimize && __optimize(fn);
+
+inspect = function() {
+  return fn(
+    {
+      cond: true,
+    },
+    { a: 0, b: 0 }
+  );
+};