Skip to content

Destructuring of arrow function arguments via computed property throws in v6.x #10347

Description

@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

Activity

  1. vsemozhetbyt commented on Dec 19, 2016

    @vsemozhetbyt
    Contributor

    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());
  2. oaleynik commented on Dec 19, 2016

    @oaleynik
    Author

    @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.

  3. vsemozhetbyt commented on Dec 19, 2016

    @vsemozhetbyt
    Contributor

    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);
  4. changed the title [-]Destructuring of function arguments via computed property throws[/-] [+]Destructuring of arrow function arguments via computed property throws[/+] on Dec 19, 2016
  5. changed the title [-]Destructuring of arrow function arguments via computed property throws[/-] [+]Destructuring of arrow function arguments via computed property throws in v6.9.2[/+] on Dec 19, 2016
  6. oaleynik commented on Dec 19, 2016

    @oaleynik
    Author

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

  7. vsemozhetbyt commented on Dec 20, 2016

    @vsemozhetbyt
    Contributor

    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();
  8. oaleynik commented on Dec 20, 2016

    @oaleynik
    Author

    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?

  9. vsemozhetbyt commented on Dec 20, 2016

    @vsemozhetbyt
    Contributor

    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(

  10. vsemozhetbyt commented on Dec 20, 2016

    @vsemozhetbyt
    Contributor

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

  11. MylesBorins commented on Dec 20, 2016

    @MylesBorins
    Contributor
  12. self-assigned this
    on Dec 20, 2016
  13. changed the title [-]Destructuring of arrow function arguments via computed property throws in v6.9.2[/-] [+]Destructuring of arrow function arguments via computed property throws in v6.x[/+] on Dec 20, 2016
  14. hashseed commented on Dec 21, 2016

    @hashseed
    Member

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

  15. fhinkel commented on Dec 21, 2016

    @fhinkel
    Contributor

    @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)?

  16. hashseed commented on Dec 21, 2016

    @hashseed
    Member

    @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.

  17. targos commented on Dec 21, 2016

    @targos
    Member

    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).

  18. targos commented on Dec 21, 2016

    @targos
    Member

    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
    
  19. hashseed commented on Dec 21, 2016

    @hashseed
    Member

    Looks like a gclient sync issue.

  20. targos commented on Dec 21, 2016

    @targos
    Member

    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

  21. added a commit that references this issue on Dec 21, 2016
  22. targos commented on Dec 21, 2016

    @targos
    Member
  23. added a commit that references this issue on Dec 26, 2016
  24. fhinkel commented on Jan 20, 2017

    @fhinkel
    Contributor

    Looks like @targos fixed it.

  25. targos commented on Jan 20, 2017

    @targos
    Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

v8 engineIssues and PRs related to the V8 dependency.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions