Node: Destructuring of arrow function arguments via computed property throws in v6.x

[oaleynik]

• node -v: v6.x
• uname -a: Darwin Olegs-MacBook-Pro.local 16.4.0 Darwin Kernel Version 16.4.0: Wed Dec 7 12:06:26 PST 2016; root:xnu-3789.41.1~5/RELEASE_X86_64 x86_64

// Throws in Nodejs 6.x with -> ReferenceError: y is not defined
var y = 'a';
var g20 = ({[y]: x}) => { var y = 'b'; return x; };
require('assert').equal(1, g20({a: 1, b: 2}));

The test case is taken from: https://github.com/nodejs/node/blob/v6.x/deps/v8/test/mjsunit/es6/destructuring.js#L893

[vsemozhetbyt]

According to node.green it should work in 6.9.0–6.9.2. This example from "computed properties" tooltip there works fine in the 6.9.2

function test(){
  var qux = "corge";
  return function({ [qux]: grault }) {
    return grault === "garply";
  }({ corge: "garply" });
}

console.log(test());

[oaleynik]

@vsemozhetbyt example from node.green indeed works. But the same example, slightly changed, throws:

const qux = "corge";
const fn = ({ [qux]: grault }) => {
  return grault === "garply";
};

console.log(fn({ corge: "garply" }));

UPDATE: What is interesting - if replace arrow function with the regular function - it works.

[vsemozhetbyt]

So this is corrected formulation: destructuring of arrow function arguments via computed property throws in the Node.js 6.9.2:

// does not throw in the Node.js 6.9.2
const obj = { key: 'value' };

const keyComputed = 'key';

function fn({ [keyComputed]: param }) {}

fn(obj);

// throws in the Node.js 6.9.2
const obj = { key: 'value' };

const keyComputed = 'key';

const fn = ({ [keyComputed]: param }) => {};

fn(obj);

[oaleynik]

Thanks @vsemozhetbyt, I've updated the issue title and description.

[vsemozhetbyt]

It seems the bug is actual only for destructuring. Other evaluations in the parameters scope are OK.

// does not throw in the Node.js 6.9.2
const obj = { key: 'value' };

const keyComputed = 'key';

function fn(param = obj[keyComputed]) { console.log(param); }

fn();

// does not throw in the Node.js 6.9.2
const obj = { key: 'value' };

const keyComputed = 'key';

const fn = (param = obj[keyComputed]) => { console.log(param); };

fn();

[oaleynik]

I see the test case for this in V8 on 6.x branch: https://github.com/nodejs/node/blob/v6.x/deps/v8/test/mjsunit/es6/destructuring.js#L893

Should not it fail?

[vsemozhetbyt]

Well, this throws:

// throws in the Node.js 6.9.2
var y = 'a';
var g20 = ({[y]: x}) => { var y = 'b'; return x; };
require('assert').equal(1, g20({a: 1, b: 2}));

I don't know why it does not fail there(

[vsemozhetbyt]

Maybe these tests are not actually tested during Node.js build (unlike Node tests)?

[MylesBorins]

the V8 tests are not run during the normal node CI

On Mon, Dec 19, 2016, 7:50 PM Vse Mozhet Byt [e-mail]
wrote:

Maybe these tests are not actually tested during Node.js build (unlike
Node tests https://github.com/nodejs/node/tree/master/test)?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/10347#issuecomment-268122297, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV_qmc70Sg0sn2EgYzPqz3CeYCqAhks5rJyZdgaJpZM4LRRZ2
.

[hashseed]

This seems to work correctly on the newest V8 (version 5.7).
@fhinkel is there any merit in bisecting what the fix was?

[fhinkel]

@hashseed I think this is a fix we want to backport. Do I read it correctly that it was broken because of some interaction with Node (because V8 has a test for it and it's not a regression test)?

[hashseed]

@fhinkel I'm under the impression that it's just broken due to V8, and should be reproducible on d8 at an older version. I haven't tested though.

[targos]

Just compared node with d8 (V8 5.1.281.89) both built from v6.x-staging and the following test:

(function(){
const qux = "corge";
const fn = ({ [qux]: grault }) => {
  return grault === "garply";
};

var _print = typeof console === 'undefined' ? print : console.log;

_print(fn({ corge: "garply" }));
})();

Test fails with both, only when the code is wrapped in an IIFE (simulating our module wrapper).

[targos]

The bug is already fixed in 5.2-lkgr. I'm trying to do a bisect but I get the following error at the end of the build:

make[1]: Entering directory '/home/mzasso/git/chromium/v8/v8/out'
  LINK(target) /home/mzasso/git/chromium/v8/v8/out/x64.release/shell
  LINK(target) /home/mzasso/git/chromium/v8/v8/out/x64.release/hello-world
  LINK(target) /home/mzasso/git/chromium/v8/v8/out/x64.release/process
  LINK(target) /home/mzasso/git/chromium/v8/v8/out/x64.release/d8
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o: unsupported reloc 41 against global symbol __libc_start_main
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o: unsupported reloc 42 against global symbol __gmon_start__
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o: unsupported reloc 41 against global symbol __libc_start_main
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o: unsupported reloc 42 against global symbol __gmon_start__
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o: unsupported reloc 41 against global symbol __libc_start_main
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o: unsupported reloc 42 against global symbol __gmon_start__
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o: unsupported reloc 41 against global symbol __libc_start_main
/home/mzasso/git/chromium/v8/v8/third_party/binutils/Linux_x64/Release/bin/ld.gold: error: /usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o: unsupported reloc 42 against global symbol __gmon_start__
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o(.init+0x7): error: unsupported reloc 42
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o(.init+0x7): error: unsupported reloc 42
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o:function _start: error: unsupported reloc 41
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o:function _start: error: unsupported reloc 41
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o(.init+0x7): error: unsupported reloc 42
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o:function _start: error: unsupported reloc 41
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crti.o(.init+0x7): error: unsupported reloc 42
/usr/lib/gcc/x86_64-redhat-linux/6.2.1/../../../../lib64/crt1.o:function _start: error: unsupported reloc 41
clang: error: linker command failed with exit code 1 (use -v to see invocation)
samples/hello-world.target.x64.release.mk:265: recipe for target '/home/mzasso/git/chromium/v8/v8/out/x64.release/hello-world' failed
make[1]: *** [/home/mzasso/git/chromium/v8/v8/out/x64.release/hello-world] Error 1
make[1]: *** Waiting for unfinished jobs....
clang: error: linker command failed with exit code 1 (use -v to see invocation)
samples/shell.target.x64.release.mk:265: recipe for target '/home/mzasso/git/chromium/v8/v8/out/x64.release/shell' failed
make[1]: *** [/home/mzasso/git/chromium/v8/v8/out/x64.release/shell] Error 1
clang: error: linker command failed with exit code 1 (use -v to see invocation)
samples/process.target.x64.release.mk:265: recipe for target '/home/mzasso/git/chromium/v8/v8/out/x64.release/process' failed
make[1]: *** [/home/mzasso/git/chromium/v8/v8/out/x64.release/process] Error 1
clang: error: linker command failed with exit code 1 (use -v to see invocation)
src/d8.target.x64.release.mk:267: recipe for target '/home/mzasso/git/chromium/v8/v8/out/x64.release/d8' failed
make[1]: *** [/home/mzasso/git/chromium/v8/v8/out/x64.release/d8] Error 1
make[1]: Leaving directory '/home/mzasso/git/chromium/v8/v8/out'
Makefile:312: recipe for target 'x64.release' failed
make: *** [x64.release] Error 2

[hashseed]

Looks like a gclient sync issue.

[targos]

Still happens after removing binutils and syncing again.
Fixed with ln -fs /usr/bin/ld.gold ./third_party/binutils/Linux_x64/Release/bin/ld.gold

[targos]

https://github.com/nodejs/node/pull/10386

[fhinkel]

Looks like @targos fixed it.

[targos]

Thanks. I forgot that we can only close issues by pushing on master!