deps: update V8 to 6.2 #15362

targos · 2017-09-12T11:20:17Z

I'm opening this early to track the issues that need fixing.

Blocking issues

Compilation failure with Clang (macOS, FreeBSD)

Error

In file included from ../deps/v8/src/compiler/common-operator.cc:5:
In file included from ../deps/v8/src/compiler/common-operator.h:8:
In file included from ../deps/v8/src/assembler.h:38:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/forward_list:173:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/memory:602:
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/utility:318:37: error: no type named 'type' in 'std::__1::enable_if<false, void>'; 'enable_if' cannot be used to disable this declaration
                 typename enable_if<is_convertible<_U1, _T1>::value &&
                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/common-operator.h:137:62: note: in instantiation of member function 'std::__1::pair<unsigned int, const v8::internal::ZoneVector<v8::internal::MachineType> *>::pair' requested here
  using std::pair<uint32_t, const ZoneVector<MachineType>*>::pair;
                                                             ^
../deps/v8/src/base/functional.h:143:69: note: while substituting deduced template arguments into function template 'TypedObjectStateInfo' [with _U1 = unsigned int, _U2 = int]
  V8_INLINE size_t operator()(T const& v) const { return hash_value(v); }
                                                                    ^
../deps/v8/src/compiler/operator.h:188:47: note: in instantiation of member function 'v8::base::hash<v8::internal::compiler::ObjectStateInfo>::operator()' requested here
    return base::hash_combine(this->opcode(), this->hash_(this->parameter()));
                                              ^
../deps/v8/src/compiler/operator.h:169:3: note: in instantiation of member function 'v8::internal::compiler::Operator1<v8::internal::compiler::ObjectStateInfo, v8::internal::compiler::OpEqualTo<v8::internal::compiler::ObjectStateInfo>, v8::internal::compiler::OpHash<v8::internal::compiler::ObjectStateInfo> >::HashCode' requested here
  Operator1(Opcode opcode, Properties properties, const char* mnemonic,
  ^
../deps/v8/src/compiler/common-operator.cc:1260:23: note: in instantiation of member function 'v8::internal::compiler::Operator1<v8::internal::compiler::ObjectStateInfo, v8::internal::compiler::OpEqualTo<v8::internal::compiler::ObjectStateInfo>, v8::internal::compiler::OpHash<v8::internal::compiler::ObjectStateInfo> >::Operator1' requested here
  return new (zone()) Operator1<ObjectStateInfo>(  // --
                      ^

Windows test failure

~~https://ci.nodejs.org/job/node-test-binary-windows/11061/RUN_SUBSET=0,VS_VERSION=vs2015,label=win2008r2/console~~

~~This one is because we do not support building addons with VS2013 anymore. We need to stop testing this on Node >=9.0.0.~~

targos · 2017-09-12T11:21:55Z

CI: https://ci.nodejs.org/job/node-test-pull-request/10043/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/900/

SteveCruise · 2017-09-12T13:48:04Z

Greate！

vsemozhetbyt · 2017-09-12T14:09:03Z

Will it be semver-major or landed on Node.js 8 LTS?

targos · 2017-09-12T15:19:19Z

Semver-major

targos · 2017-09-13T22:53:48Z

I updated the OP with two main issues.

/cc @nodejs/build @nodejs/platform-macos @nodejs/platform-freebsd

cjihrig · 2017-09-14T00:05:46Z

FYI: I built this branch and ran the llnode test suite. All tests passed.

MylesBorins · 2017-09-14T13:15:02Z

@targos I've gone ahead and put in request to get all floated patches upstreamed / landed on 6.2

https://chromium-review.googlesource.com/c/v8/v8/+/666925
https://chromium-review.googlesource.com/c/v8/v8/+/666961

https://bugs.chromium.org/p/v8/issues/detail?id=6824
https://bugs.chromium.org/p/v8/issues/detail?id=6825
https://bugs.chromium.org/p/v8/issues/detail?id=6826

MylesBorins · 2017-09-14T13:29:05Z

One more CI job to see if the gclient thing is a real problem or not

https://ci.nodejs.org/job/node-test-commit-v8-linux/904/

MylesBorins · 2017-09-14T13:35:36Z

Here is an ABI-Smoker job being run against this current PR... it should break. If it works... we have a problem

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-abi-smoker/39/

refack · 2017-09-14T14:13:28Z

This one is because we do not support building addons with VS2013 anymore. We need to stop testing this on Node >=9.0.0.

@targos do you think the problems on VS_VERSION=vs2015-x86,label=win2008r2 is a bug in the build script?

joaocgreis · 2017-09-14T18:00:44Z

I'll update CI to not use VS2013 for Node 9.

MylesBorins · 2017-09-14T21:25:42Z

@mhdawson any idea why ci is breaking during gsync?

targos · 2017-09-18T12:10:41Z

Updated and removed the V8 backports. I think some of them still need to be merged to 6.2?

addaleax · 2017-09-18T19:45:02Z

@targos Which ones? If you’re talking about the ones from #15391, yeah, as internal refactoring + an API change I don’t think they would be eligible for merging into 6.2. Feel free to ask for it if you think otherwise…

targos · 2017-09-18T19:53:28Z

I'm talking about the ones linked in #15362 (comment)

targos · 2017-09-19T08:17:17Z

@addaleax I cherry-picked 8403d6b#diff-ff06109b32824fc20fec697f584a42e3 to this PR. Your other cherry-pick is already in 6.2.

targos · 2017-09-19T14:05:38Z

Does anyone have an idea about how we can fix the error with Clang?

MylesBorins · 2017-09-19T18:18:32Z

CI: https://ci.nodejs.org/job/node-test-pull-request/10137/
V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/916/

Haven't seen clang errors yet, perhaps they'll surface in above jobs

targos · 2017-09-19T19:41:31Z

You can see the error in the FreeBSD job. OSX is still pending.

joaocgreis · 2017-09-20T01:25:48Z

CI changed to not run Node >=9 on VS2013. This fixed the Windows issue in this PR. Here is a Windows CI run: https://ci.nodejs.org/job/node-test-commit-windows-fanned/11804/

vsemozhetbyt · 2017-09-22T22:15:20Z

Refs: https://v8project.blogspot.com/2017/09/disabling-escape-analysis.html

With V8 6.2 there is one line less in the promise trace. PR-URL: nodejs#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: nodejs#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original commit message: avoid constructor inheritance due to compilation issues Constructor inheritance of a templated constructor is causing compilation issues for node.js: https: //github.com/nodejs/pull/15362#issue-257007421 Change-Id: I7d099ff5a1a2fd5b19c11112ddef8fe824e509f7 Reviewed-on: https://chromium-review.googlesource.com/707008 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#48445} Refs: v8/v8@0f1dfae PR-URL: nodejs#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

With V8 6.2 there is one line less in the promise trace. PR-URL: #15362 Backport-PR-URL: #16413 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #15362 Backport-PR-URL: #16413 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original commit message: avoid constructor inheritance due to compilation issues Constructor inheritance of a templated constructor is causing compilation issues for node.js: https: //github.com//pull/15362#issue-257007421 Change-Id: I7d099ff5a1a2fd5b19c11112ddef8fe824e509f7 Reviewed-on: https://chromium-review.googlesource.com/707008 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#48445} Refs: v8/v8@0f1dfae PR-URL: #15362 Backport-PR-URL: #16413 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

nodejs-github-bot added tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Sep 12, 2017

targos added wip Issues and PRs that are still a work in progress. and removed tools Issues and PRs related to the tools directory. labels Sep 12, 2017

MylesBorins added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 12, 2017

targos force-pushed the v8-6.2 branch from 067fab1 to ec16c50 Compare September 13, 2017 22:53

joaocgreis mentioned this pull request Sep 14, 2017

Tracking issue: Change 4 Windows 2008 machines to Visual Studio 2017 nodejs/build#883

Closed

vsemozhetbyt mentioned this pull request Sep 18, 2017

[v8.x] deps: backport V8 update to 6.1 #15393

Closed

2 tasks

targos force-pushed the v8-6.2 branch from ec16c50 to a5054d9 Compare September 18, 2017 12:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update V8 to 6.2 #15362

deps: update V8 to 6.2 #15362

targos commented Sep 12, 2017 •

edited

Loading

targos commented Sep 12, 2017

SteveCruise commented Sep 12, 2017

vsemozhetbyt commented Sep 12, 2017

targos commented Sep 12, 2017

targos commented Sep 13, 2017

cjihrig commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

refack commented Sep 14, 2017

joaocgreis commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

targos commented Sep 18, 2017

addaleax commented Sep 18, 2017

targos commented Sep 18, 2017

targos commented Sep 19, 2017

targos commented Sep 19, 2017 •

edited

Loading

MylesBorins commented Sep 19, 2017

targos commented Sep 19, 2017

joaocgreis commented Sep 20, 2017

vsemozhetbyt commented Sep 22, 2017

deps: update V8 to 6.2 #15362

deps: update V8 to 6.2 #15362

Conversation

targos commented Sep 12, 2017 • edited Loading

Blocking issues

Compilation failure with Clang (macOS, FreeBSD)

Windows test failure

targos commented Sep 12, 2017

SteveCruise commented Sep 12, 2017

vsemozhetbyt commented Sep 12, 2017

targos commented Sep 12, 2017

targos commented Sep 13, 2017

cjihrig commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

refack commented Sep 14, 2017

joaocgreis commented Sep 14, 2017

MylesBorins commented Sep 14, 2017

targos commented Sep 18, 2017

addaleax commented Sep 18, 2017

targos commented Sep 18, 2017

targos commented Sep 19, 2017

targos commented Sep 19, 2017 • edited Loading

MylesBorins commented Sep 19, 2017

targos commented Sep 19, 2017

joaocgreis commented Sep 20, 2017

vsemozhetbyt commented Sep 22, 2017

targos commented Sep 12, 2017 •

edited

Loading

targos commented Sep 19, 2017 •

edited

Loading