Node: V8 Backports Triage

[ofrobots]

I am going through the list of V8 fixes that potentially need a backport for Node.js. These would be good first contributions / investigations.

• [ ] https://bugs.chromium.org/p/v8/issues/detail?id=5301. Affects V8 5.1, so needed for v6.x.
• [ ] https://bugs.chromium.org/p/v8/issues/detail?id=5340: @joransiu: is this needed for v6.x?. Not needed.
• [ ] https://bugs.chromium.org/p/v8/issues/detail?id=5363: Possibly needed. Need to investigate if the bug is present in v6.x and v4.x. Not needed.

/cc @nodejs/lts, @nodejs/v8.

[caitp]

https://bugs.chromium.org/p/v8/issues/detail?id=5363 is not present in v6.x or lower (the old JS implementation did not have this bug, and the fix is already applied when the C++ version was pulled in in September).

[john-yan]

https://bugs.chromium.org/p/v8/issues/detail?id=5340 is not needed in 5.1.

[maasencioh]

I'm interested in help with this

[ofrobots]

@maasencioh Great! You should consult with the guide in https://github.com/nodejs/LTS/pull/137. See section on backporting to abandoned branches. I'd be happy to help you through the process.

[maasencioh]

Thanks @ofrobots, I was looking at the issue, and it was solved in this commit https://github.com/v8/v8/commit/5af4cd984067f4e316aea04ef381a00724b5a8bf
I wonder if it actually has sense to backport it to v6, because tail calls are not supported in this version

[ofrobots]

@maasencioh By backtails do you mean 'tail calls'? They are not officially supported w/ Node.js v6.x, but are available under a flag. It is indeed not critical to fix this issue, but if the fix is simple enough there is no harm in backporting.

[targos]

@ofrobots https://github.com/v8/v8/commit/5af4cd984067f4e316aea04ef381a00724b5a8bf only adds tests for tail call expressions like return continue f();. The change in src/parsing/parser.cc is easy to backport but do you know of a regression test that could be added for it ?

[targos]

ping @ofrobots ^

[ofrobots]

@littledan: ^^ do you have a suggestion for a test for the change in praser.(h|cc)?

[littledan]

The test in test/mjsunit/regress/regress-639270.js is a regression test against implicit tail calls, and the tests in test/mjsunit/es8/syntactic-tail-call-parsing.js and test/message/syntactic-tail-call-generator.js check for explicit tail calls. Note however that none of this affects the default configuration; you'd need to pass experimental flags to trigger the bug path.

[Trott]

Should this remain open?

[ofrobots]

The only remaining issue here is v8:5301. Given that the bug doesn't manifest in the default configuration (--harmony-tailcalls is needed), I would say that this doesn't necessarily need a backport. I am closing the issue.