Repository navigation
V8 Backports Triage #9190
Description
Activity
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).
Reacted by Ali Ijaz Sheikhhttps://bugs.chromium.org/p/v8/issues/detail?id=5340 is not needed in 5.1.
Reacted by Ali Ijaz SheikhI'm interested in help with this
@maasencioh Great! You should consult with the guide in nodejs/Release#137. See section on backporting to abandoned branches. I'd be happy to help you through the process.
Thanks @ofrobots, I was looking at the issue, and it was solved in this commit v8/v8@5af4cd9
I wonder if it actually has sense to backport it to v6, because tail calls are not supported in this version@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.@ofrobots v8/v8@5af4cd9 only adds tests for tail call expressions like
return continue f();. The change insrc/parsing/parser.ccis easy to backport but do you know of a regression test that could be added for it ?@littledan: ^^ do you have a suggestion for a test for the change in
praser.(h|cc)?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.
Should this remain open?
I am going through the list of V8 fixes that potentially need a backport for Node.js. These would be good first contributions / investigations.
v6.x.https://bugs.chromium.org/p/v8/issues/detail?id=5340: @joransiu: is this needed for. Not needed.v6.x?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.