Adding ElementsAccessor::Concat

Adding ElementsAccessor::Concat
- Moving parts of ArrayConcat from builtins.cc to the ElementsAccessor
- Removing ArrayConcat Runtime Function

BUG=v8:4317
LOG=N

Committed: https://crrev.com/cbdb13533ee6c5fdd2ed5df41e70892cb25e5f57
Cr-Commit-Position: refs/heads/master@{#30619}

[Camillo Bruni, 2015-09-04 09:49:46]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[Michael Achenbach, 2015-09-04 10:54:07]

On 2015/09/04 09:49:46, cbruni wrote:
> The CQ bit was checked by mailto:cbruni@chromium.org to run a CQ dry run

CQ down - trooper informed.

[commit-bot: I haz the power, 2015-09-04 10:56:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330483003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330483003/120001

[commit-bot: I haz the power, 2015-09-04 11:08:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 11:08:47]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/9348)

[Camillo Bruni, 2015-09-04 13:54:54]

cbruni@chromium.org changed reviewers:
+ ishell@chromium.org

[Camillo Bruni, 2015-09-04 13:54:55]

PTAL

https://codereview.chromium.org/1330483003/diff/120001/src/array.js
File src/array.js (right):

https://codereview.chromium.org/1330483003/diff/120001/src/array.js#newcode525
src/array.js:525: function ArrayConcatJS(arg1) {  // length == 1
This is no longer needed, however, I don't know how to properly remove it, since
it complains during mksnapshot and/or linking.

https://codereview.chromium.org/1330483003/diff/120001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1330483003/diff/120001/src/builtins.cc#newcod...
src/builtins.cc:622: class ArrayConcatVisitor {
Probably ElementsAccessor would be a better place for this?

https://codereview.chromium.org/1330483003/diff/120001/src/builtins.cc#newcod...
src/builtins.cc:1177: bool HasConcatSpreadableModifier(Isolate* isolate,
Handle<Object> obj) {
At some point we need something faster here, since we have a massive penalty
with the FLAG_harmony_concat_spreadable enabled. At least we are more correct
this way (see https://codereview.chromium.org/1247243003)

https://codereview.chromium.org/1330483003/diff/120001/src/builtins.cc#newcod...
src/builtins.cc:1409: if (HasConcatSpreadableModifier(isolate, array)) {
Needed for certain cases (see https://codereview.chromium.org/1247243003)

https://codereview.chromium.org/1330483003/diff/120001/src/runtime/runtime-ar...
File src/runtime/runtime-array.cc (right):

https://codereview.chromium.org/1330483003/diff/120001/src/runtime/runtime-ar...
src/runtime/runtime-array.cc:448: RUNTIME_FUNCTION(Runtime_ArrayConcat) {
Again, I need to see on how to remove this.

[Camillo Bruni, 2015-09-04 16:01:44]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 16:01:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330483003/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330483003/200001

[commit-bot: I haz the power, 2015-09-04 16:27:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 16:27:25]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2015-09-07 10:53:11]

lgtm with nits:

https://codereview.chromium.org/1330483003/diff/200001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1330483003/diff/200001/src/bootstrapper.cc#ne...
src/bootstrapper.cc:2341: // Make sure that Function.prototype.call appears to
be compiled.
Please update comment.

https://codereview.chromium.org/1330483003/diff/200001/src/bootstrapper.cc#ne...
src/bootstrapper.cc:2359: // Make sure that Function.prototype.call appears to
be compiled.
Please update comment.

https://codereview.chromium.org/1330483003/diff/200001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1330483003/diff/200001/src/builtins.cc#newcod...
src/builtins.cc:1178: if (!obj->IsJSReceiver()) return false;
You call this only for JSArray objects. If you update the signature, you can
remove this check.

https://codereview.chromium.org/1330483003/diff/200001/src/builtins.cc#newcod...
src/builtins.cc:1422: isolate->Throw(*isolate->factory()->NewRangeError(
Please add a comment here why do we throw here.
You can write
  return isolate->Throw<JSArray>(*isolate->factory()->...);

[Camillo Bruni, 2015-09-07 13:01:46]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2015-09-07 13:01:46]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/1330483003/#ps240001
(title: "merging master")

[commit-bot: I haz the power, 2015-09-07 13:01:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330483003/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330483003/240001

[commit-bot: I haz the power, 2015-09-07 13:44:51]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2015-09-07 13:45:12]

Patchset 13 (id:??) landed as
https://crrev.com/cbdb13533ee6c5fdd2ed5df41e70892cb25e5f57
Cr-Commit-Position: refs/heads/master@{#30619}