New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 2700 link

Starred by 258 users

Issue metadata

Status: Fixed
Owner:
User never visited
Closed: Jul 2015
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 1
Type: FeatureRequest


Sign in to add a comment

Implement arrow functions

Reported by farid...@gmail.com, May 28 2013

Issue description

Cc: mstarzinger@chromium.org
Labels: Type-FeatureRequest Harmony
Owner: rossberg@chromium.org
Status: Assigned
What's wrong with the actual language construct? What benefit does it have to be able to write `foo(x => x + 1);` instead of`foo(function(x) { return x + 1; });`, other than saving a few bytes and losing verbosity (i.e. clarity) in the code?

Comment 3 by olos...@gmail.com, Nov 21 2013

For me the main benefits would be:
1. Easier to read. Today mostly all modern languages has this syntax: (C# http://msdn.microsoft.com/en-us/library/vstudio/bb397687.aspx,  Dart: https://www.dartlang.org/docs/spec/latest/dart-language-specification.html#h.kt174mfrzv4a ). Some other very close to that. But in general this is much easier/faster to read:
[1,2,3].map(x => x*2).filter(x => x < 4)
compared to
[1,2,3].map(function(x) { return x*2}).filter(function(x) { return x < 4})

2. Just faster to write code. Even using some advanced editors you still need to press more keystrokes. Why not to make life little bit easier?

3. Now Firefox already supports this syntax even in stable versions. By implementing this in V8 it will allow to write the same code at least for both Firefox and Chrome/Node.JS

Of Course it could be abused, but for simple expressions this would be a great feature to have. Waiting this feature to be released.

Comment 4 by rossberg@google.com, Nov 21 2013

@ #2: one important feature of arrow functions is that they have "lexical 'this'", i.e., 'this' is taken from the surrounding scope. This is far more convenient in a lot of cases, and avoids the common (and error-prone) "var that = this" pattern.
The time for debate about whether or not to add this feature to the language is passed, and in any case the place for that was on the es-discuss mailing list. At this point it's just an implementation tracking bug. It is already implemented in SpiderMonkey and is shipping in the release channel of Firefox. As #3 points out, it would be fantastic for v8 to implement this for both Chrome and Node.js. Some parts of ES6 are still a moving target, but this feature has been stable and has reached concensus in TC39 for over a year. As such, implementation should be prioritized over other ES6 features.

Comment 6 by olos...@gmail.com, Dec 1 2013

JFI current implementation of arrow functions in SpiderMonkey has a huge performance problem:
http://jsperf.com/arrow-functions-this-usage
http://jsperf.com/simple-arrow-functions

But actually as it is more like syntax sugar, it should not affect performance (even with "that=this" trick). 

If somebody is working on it in V8 please consider that performance here is quite serious problem that could be a blocking factor from using this feature (like it is  now with SpiderMonkey
The implementation you're referring to is still very much in its infancy and will most certainly be flawed. The real question is whether or not you bothered to file a bug for SpiderMonkey—if so, please post here.
I am currently working on implementing support for arrow functions
behind an harmony_arrow_functions flag. Parsing and execution of
arrow functions with lazy compilation disabled is already working,
so I expect to start submitting patches for review soon.

WIP branch: https://github.com/aperezdc/v8/tree/arrow-functions
Review request for arrow-function parsing:
  https://codereview.chromium.org/160063003/

* Parsing works and returns a FunctionLiteral.
* Invoking arrow-functions works if “--no-lazy” is passed to d8.
* Proper semantics missing (mainly the binding of “this”).

Comment 10 by dil...@gmail.com, Feb 22 2014

No issue exists with that id (160063003)
Cc: dslomov@chromium.org
Summary: Implement arrow functions (was: Implement harmony arrow function syntax)

Comment 15 by m.go...@gmail.com, Jun 18 2014

The performance penalty in SpiderMonkey was caused by the fact it was implemented as a sugar for `function (...) {...}.bind(this)` and bind is slow in SpiderMonkey. The implementation was recently changed to use an internal function slot for storing the `this` value and now (since Firefox 31) arrow functions are fast.
Arrow functions in canary can be bound (39.0.2135.0 canary (64-bit)), which may be contrary to the draft specification. Arrow functions cannot be bound in spidermonkey, so there is at least an inconsistency between it and V8. I wrote a little example in this gist:

https://gist.github.com/qubyte/43e0093274e793cc82ba


> which may be contrary to the draft specification.

This is absolutely contrary to the specification. 
In fact, this is mentioned specifically in https://people.mozilla.org/~jorendorff/es6-draft.html#sec-function.prototype.bind, note 2
Cc: ape...@igalia.com
Relax, arrow functions are work in progress. They do not have their own code generation yet and are simply compiled as ordinary functions as a dummy choice (i.e., scoping of `this` and `arguments` also does not work).

Consider a feature complete once it moves from the --harmony to the --es-staging flag.

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 12 2014

The following revision refers to this bug:
  https://chromium.googlesource.com/external/v8.git/+/477b75f1cb2c21a55a8c96ccefbfb19ee1715336

commit 477b75f1cb2c21a55a8c96ccefbfb19ee1715336
Author: arv@chromium.org <arv@chromium.org>
Date: Fri Sep 12 15:07:43 2014

Arrow functions: Cleanup handling of the prototype property

The old code did not work correctly in case of optimizations. I
found this out when implementing concise methods and we now plumb
through the function kind so we know what kind of Map to create for
the function.

BUG= v8:2700 
LOG=y
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/562253002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23920 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

[modify] https://chromium.googlesource.com/external/v8.git/+/477b75f1cb2c21a55a8c96ccefbfb19ee1715336/src/contexts.h
[modify] https://chromium.googlesource.com/external/v8.git/+/477b75f1cb2c21a55a8c96ccefbfb19ee1715336/src/factory.cc
[modify] https://chromium.googlesource.com/external/v8.git/+/477b75f1cb2c21a55a8c96ccefbfb19ee1715336/test/mjsunit/harmony/arrow-functions.js

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 16 2014

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5

commit 0841f7241b2c36904f781b71859bf3f77a854af5
Author: wingo@igalia.com <wingo@igalia.com>
Date: Thu Oct 16 13:19:36 2014

Track usage of "this" and "arguments" in Scope

This adds flags in Scope to track wheter a Scope uses "this" and,
"arguments". The information is exposed via Scope::uses_this(),
and Scope::uses_arguments(), respectively. Flags for tracking
usage on any inner scope uses are available as well via
Scope::inner_uses_this(), and Scope::inner_uses_arguments().

Knowing whether scopes use "this" and "arguments" will be handy
to generate the code needed to capture their values when generating
the code for arrow functions.

BUG= v8:2700 
LOG=
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/422923004

Patch from Adrian Perez de Castro <aperez@igalia.com>.

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24663 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/ast-value-factory.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/globals.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/objects.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/preparser.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/runtime/runtime-debug.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopeinfo.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopes.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopes.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/test/cctest/test-parsing.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 16 2014

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5

commit 0841f7241b2c36904f781b71859bf3f77a854af5
Author: wingo@igalia.com <wingo@igalia.com>
Date: Thu Oct 16 13:19:36 2014

Track usage of "this" and "arguments" in Scope

This adds flags in Scope to track wheter a Scope uses "this" and,
"arguments". The information is exposed via Scope::uses_this(),
and Scope::uses_arguments(), respectively. Flags for tracking
usage on any inner scope uses are available as well via
Scope::inner_uses_this(), and Scope::inner_uses_arguments().

Knowing whether scopes use "this" and "arguments" will be handy
to generate the code needed to capture their values when generating
the code for arrow functions.

BUG= v8:2700 
LOG=
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/422923004

Patch from Adrian Perez de Castro <aperez@igalia.com>.

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24663 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/ast-value-factory.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/globals.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/objects.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/preparser.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/runtime/runtime-debug.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopeinfo.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopes.cc
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/src/scopes.h
[modify] https://chromium.googlesource.com/v8/v8.git/+/0841f7241b2c36904f781b71859bf3f77a854af5/test/cctest/test-parsing.cc

It appears that chromium 38 has half of an arrow function implementation.  You can define arrow functions but they don't have a lexical this.

function Foo() { this.x = true };
Foo.prototype.bound = function(){ return () => this.x }

var foo = new Foo();
foo.bound()(); //=> undefined.

The last line should return true.
#23: see my earlier comment (#19). This is work in progress and behind a flag. Don't expect it to do anything sensible yet.

No problem, I'm just pointing it out to hopefully save a little confusion.

Comment 26 by olos...@gmail.com, Nov 23 2014

Was this functionality removed? In 41.0.2227.1 canary there is no arrow function support even with enabled Harmony flag (chrome://flags/#enable-javascript-harmony). 

Is there any way to bring it back? We don't expect to work perfectly but it would be great to start playing with it right now, so when it become stable, our code would be prepared to it.

Comment 27 by i...@bnoordhuis.nl, Nov 23 2014

Arrow functions are behind the --harmony_arrow_functions flag.  I suspect that the reason --harmony doesn't enable them, is that they are really buggy right now.  For example, `let id = x => x; id(42)` triggers an assert in the parser.
Arrow functions are work in progress and not yet usable. To remove any confusion about that, we removed this and a few other in-progress features from being implied by the --harmony flag, until they are actually completed. 

@ #27: Hm, I cannot reproduce any problem with that example.

Comment 29 by i...@bnoordhuis.nl, Nov 25 2014

@ #28: still happens for me with today's HEAD of master, commit f0c9cc0.

$ git log --oneline -1 HEAD
f0c9cc0 MIPS: Leaving a generator via an exception causes it to close.

$ make -j8 x64.debug extrachecks=on i18nsupport=off
[elided]

$ out/x64.debug/d8 --harmony --harmony_arrow_functions --use_strict
V8 version 3.31.0 (candidate) [console: dumb]
d8> let f = x => x; f(42)


#
# Fatal error in .././src/scopes.h, line 152
# CHECK(!already_resolved()) failed
#

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::VariableProxy* v8::internal::Scope::NewUnresolved<v8::internal::AstConstructionVisitor>(v8::internal::AstNodeFactory<v8::internal::AstConstructionVisitor>*, v8::internal::AstRawString const*, v8::internal::Interface*, int)
 3: v8::internal::ParserTraits::ExpressionFromIdentifier(v8::internal::AstRawString const*, int, v8::internal::Scope*, v8::internal::AstNodeFactory<v8::internal::AstConstructionVisitor>*)
 4: v8::internal::ParserBase<v8::internal::ParserTraits>::ParsePrimaryExpression(bool*)
 5: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseMemberExpression(bool*)
 6: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseMemberWithNewPrefixesExpression(bool*)
 7: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseLeftHandSideExpression(bool*)
 8: v8::internal::ParserBase<v8::internal::ParserTraits>::ParsePostfixExpression(bool*)
 9: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseUnaryExpression(bool*)
10: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseBinaryExpression(int, bool, bool*)
11: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseConditionalExpression(bool, bool*)
12: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseAssignmentExpression(bool, bool*)
13: v8::internal::ParserBase<v8::internal::ParserTraits>::ParseExpression(bool, bool*)
14: v8::internal::Parser::ParseLazy(v8::internal::Utf16CharacterStream*)
15: v8::internal::Parser::ParseLazy()
16: v8::internal::Parser::Parse()
17: v8::internal::Parser::Parse(v8::internal::CompilationInfo*, bool)
18: ??
19: v8::internal::Compiler::GetLazyCode(v8::internal::Handle<v8::internal::JSFunction>)
20: ??
21: v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*)
22: ??

Illegal instruction (core dumped)

I can file a new issue if you want.
Ah, thanks, I can indeed reproduce it in debug mode.

Adrian, can you have a look?

Comment 31 by ape...@igalia.com, Nov 27 2014

This seems to be the same problem as reported in  issue #3501 , in this case
the stack trace is not exactly the same, but I think this will be gone when
I get the other bug squashed.
Blockedon: chromium:465671
ClusterFuzz found  issue chromium:465671 .
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 24 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/00844d466be3d809b95d439fdd14566601780458

commit 00844d466be3d809b95d439fdd14566601780458
Author: aperez <aperez@igalia.com>
Date: Tue Mar 24 13:08:26 2015

Cleanups needed for this-scoping in arrow functions

Remove Variable::IsValidReference(), and the Variable::is_valid_ref_
member: This was "false" only for "this", and for internal variables.
For the first, VariableProxy::is_this() can be used for the check
instead; and for internal variables, it is guaranteed they they will
not be written to (because the V8 code does not do it, and they are
not accessible from JavaScript).

The "bool is_this" parameter of VariableProxy() constructor is
changed to use Variable::Kind. This will allow to later on adding
a parameter to create unresolved variables of any kind, which in
turn will be used to make references to "this" initially unresolved,
and use the existing variable resolution mechanics for "this".

BUG= v8:2700 
LOG=N

Review URL: https://codereview.chromium.org/1024703004

Cr-Commit-Position: refs/heads/master@{#27404}

[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/ast.cc
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/ast.h
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/parser.cc
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/scopes.cc
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/scopes.h
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/variables.cc
[modify] http://crrev.com/00844d466be3d809b95d439fdd14566601780458/src/variables.h

Project Member

Comment 34 by bugdroid1@chromium.org, Apr 9 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/635b5fea9879de29173376ee12ae12319370123e

commit 635b5fea9879de29173376ee12ae12319370123e
Author: arv <arv@chromium.org>
Date: Thu Apr 09 19:39:34 2015

Lexical arguments for arrow functions

Only allocate 'arguments' if the scope is not an arrow scope.

BUG= v8:2700 
LOG=N
R=adamk@chromium.org, wingo@igalia.cmo

Review URL: https://codereview.chromium.org/1078483002

Cr-Commit-Position: refs/heads/master@{#27716}

[modify] http://crrev.com/635b5fea9879de29173376ee12ae12319370123e/src/scopes.cc
[modify] http://crrev.com/635b5fea9879de29173376ee12ae12319370123e/src/scopes.h
[add] http://crrev.com/635b5fea9879de29173376ee12ae12319370123e/test/mjsunit/harmony/arrow-functions-lexical-arguments.js

Blockedon: v8:4030 v8:4031
Cc: rossberg@chromium.org
Owner: wi...@igalia.com
Labels: Area-Language Priority-High
Project Member

Comment 37 by bugdroid1@chromium.org, May 6 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/06a792b7cc2db33ffce7244c044a9c05afbb6116

commit 06a792b7cc2db33ffce7244c044a9c05afbb6116
Author: wingo <wingo@igalia.com>
Date: Wed May 06 14:18:03 2015

Resolve references to "this" the same way as normal variables

Make the parser handle references to "this" as unresolved variables, so the
same logic as for the rest of function parameters is used for the receiver.
Minor additions to the code generation handle copying the receiver to the
context, along with the rest of the function parameters.

Based on work by Adrian Perez de Castro <aperez@igalia.com>.

BUG= v8:2700 
LOG=N

Review URL: https://codereview.chromium.org/1130733003

Cr-Commit-Position: refs/heads/master@{#28263}

[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/arm/full-codegen-arm.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/arm/lithium-codegen-arm.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/arm64/full-codegen-arm64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/arm64/lithium-codegen-arm64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/compiler/ast-graph-builder.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/compiler/ast-graph-builder.h
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/contexts.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/heap/heap.h
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/ia32/full-codegen-ia32.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/ia32/lithium-codegen-ia32.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/mips/full-codegen-mips.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/mips/lithium-codegen-mips.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/mips64/full-codegen-mips64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/mips64/lithium-codegen-mips64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/parser.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/runtime/runtime-debug.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/runtime/runtime-scopes.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/scopeinfo.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/scopes.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/scopes.h
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/variables.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/x64/full-codegen-x64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/src/x64/lithium-codegen-x64.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/test/cctest/test-serialize.cc
[modify] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/test/mjsunit/debug-scopes.js
[add] http://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116/test/mjsunit/harmony/arrow-functions-this.js

Project Member

Comment 38 by bugdroid1@chromium.org, May 7 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5cab6be83a44567a3ee5747d728a399025d38411

commit 5cab6be83a44567a3ee5747d728a399025d38411
Author: machenbach <machenbach@chromium.org>
Date: Thu May 07 07:20:43 2015

Revert of Resolve references to "this" the same way as normal variables (patchset #2 id:20001 of https://codereview.chromium.org/1130733003/)

Reason for revert:
[Sheriff] Breaks jetstream benchmark with errors like this:

>>> Running suite: JetStream/bigfib.cpp
>>> Stdout (#1):
undefined:93: ReferenceError: this is not defined
  this['Module'] = Module;
  ^
ReferenceError: this is not defined
    at eval (eval at __run (runner.js:13:3), <anonymous>:93:3)
    at eval (native)
    at __run (runner.js:13:3)
    at Object.runSimpleBenchmark (runner.js:44:31)
    at runner.js:97:13

Original issue's description:
> Resolve references to "this" the same way as normal variables
>
> Make the parser handle references to "this" as unresolved variables, so the
> same logic as for the rest of function parameters is used for the receiver.
> Minor additions to the code generation handle copying the receiver to the
> context, along with the rest of the function parameters.
>
> Based on work by Adrian Perez de Castro <aperez@igalia.com>.
>
> BUG= v8:2700 
> LOG=N
>
> Committed: https://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116
> Cr-Commit-Position: refs/heads/master@{#28263}

TBR=rossberg@chromium.org,arv@chromium.org,wingo@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= v8:2700 

Review URL: https://codereview.chromium.org/1129723003

Cr-Commit-Position: refs/heads/master@{#28283}

[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/arm/full-codegen-arm.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/arm/lithium-codegen-arm.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/arm64/full-codegen-arm64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/arm64/lithium-codegen-arm64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/compiler/ast-graph-builder.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/compiler/ast-graph-builder.h
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/contexts.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/heap/heap.h
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/ia32/full-codegen-ia32.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/ia32/lithium-codegen-ia32.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/mips/full-codegen-mips.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/mips/lithium-codegen-mips.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/mips64/full-codegen-mips64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/mips64/lithium-codegen-mips64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/parser.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/runtime/runtime-debug.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/runtime/runtime-scopes.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/scopeinfo.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/scopes.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/scopes.h
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/variables.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/x64/full-codegen-x64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/src/x64/lithium-codegen-x64.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/test/cctest/test-serialize.cc
[modify] http://crrev.com/5cab6be83a44567a3ee5747d728a399025d38411/test/mjsunit/debug-scopes.js
[delete] http://crrev.com/d4f014f676a729b5ffbb8d7c774e2d3656aa1da6/test/mjsunit/harmony/arrow-functions-this.js

Project Member

Comment 39 by bugdroid1@chromium.org, May 11 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bd56d279b612f92d2d1d25f92d32fe7f42a455ca

commit bd56d279b612f92d2d1d25f92d32fe7f42a455ca
Author: wingo <wingo@igalia.com>
Date: Mon May 11 11:49:48 2015

Resolve references to "this" the same way as normal variables

Make the parser handle references to "this" as unresolved variables, so the
same logic as for the rest of function parameters is used for the receiver.
Minor additions to the code generation handle copying the receiver to the
context, along with the rest of the function parameters.

Based on work by Adrian Perez de Castro <aperez@igalia.com>.

This is a reapplication of https://codereview.chromium.org/1130733003.

R=rossberg@chromium.org
BUG= v8:2700 
LOG=N

Review URL: https://codereview.chromium.org/1136073002

Cr-Commit-Position: refs/heads/master@{#28340}

[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/arm/full-codegen-arm.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/arm/lithium-codegen-arm.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/arm64/full-codegen-arm64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/arm64/lithium-codegen-arm64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/compiler/ast-graph-builder.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/compiler/ast-graph-builder.h
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/contexts.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/heap/heap.h
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/ia32/full-codegen-ia32.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/ia32/lithium-codegen-ia32.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/mips/full-codegen-mips.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/mips/lithium-codegen-mips.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/mips64/full-codegen-mips64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/mips64/lithium-codegen-mips64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/parser.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/runtime/runtime-debug.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/runtime/runtime-scopes.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/scopeinfo.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/scopes.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/scopes.h
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/variables.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/x64/full-codegen-x64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/src/x64/lithium-codegen-x64.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/test/cctest/test-serialize.cc
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/test/mjsunit/debug-scopes.js
[add] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/test/mjsunit/harmony/arrow-functions-this.js
[modify] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/test/mjsunit/testcfg.py
[add] http://crrev.com/bd56d279b612f92d2d1d25f92d32fe7f42a455ca/test/mjsunit/this-dynamic-lookup.js

Does this mean it's landed?
It means it's (mostly) feature complete and can be turned on by the --harmony flag. It still needs some baking in Canary before we feel confident to turn it on by default.

Comment 43 by arv@chromium.org, Jun 9 2015

Not quite. It is now available under --harmony (Experimental Javascript in chrome://flags) and  it is turned on for our fuzz tester.

We are getting close though...
Project Member

Comment 44 by bugdroid1@chromium.org, Jun 10 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e7a62c24855ac2bb34271e997ffae5ffcf57862d

commit e7a62c24855ac2bb34271e997ffae5ffcf57862d
Author: wingo <wingo@igalia.com>
Date: Wed Jun 10 16:11:25 2015

Support rest parameters in arrow functions

R=dslomov@chromium.org, rossberg@chromium.org
LOG=Y
BUG= v8:2700 

Review URL: https://codereview.chromium.org/1178523002

Cr-Commit-Position: refs/heads/master@{#28908}

[modify] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/src/parser.cc
[modify] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/src/parser.h
[modify] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/src/preparser.h
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-bare-rest-param.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-bare-rest-param.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-param-after-rest-2.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-param-after-rest-2.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-param-after-rest.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-param-after-rest.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-two-rest-params.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/arrow-two-rest-params.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/invalid-spread-2.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/invalid-spread-2.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/invalid-spread.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/message/invalid-spread.out
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/mjsunit/harmony/arrow-rest-params.js
[add] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/mjsunit/harmony/regress/regress-arrow-duplicate-params.js
[modify] http://crrev.com/e7a62c24855ac2bb34271e997ffae5ffcf57862d/test/mjsunit/harmony/rest-params-lazy-parsing.js

Project Member

Comment 45 by bugdroid1@chromium.org, Jun 18 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/541b6c39e0ecae1c070f51fae8e9e3dab18d278c

commit 541b6c39e0ecae1c070f51fae8e9e3dab18d278c
Author: wingo <wingo@igalia.com>
Date: Thu Jun 18 15:13:42 2015

Ship arrow functions

R=rossberg@chromium.org
LOG=Y
BUG= v8:2700 

Review URL: https://codereview.chromium.org/1187173004

Cr-Commit-Position: refs/heads/master@{#29119}

[modify] http://crrev.com/541b6c39e0ecae1c070f51fae8e9e3dab18d278c/src/flag-definitions.h

<3 <3 <3
Project Member

Comment 47 by bugdroid1@chromium.org, Jun 18 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bf92b53ff6724d6987dda643a9263135918fe051

commit bf92b53ff6724d6987dda643a9263135918fe051
Author: machenbach <machenbach@chromium.org>
Date: Thu Jun 18 19:39:41 2015

Revert of Ship arrow functions (patchset #1 id:1 of https://codereview.chromium.org/1187173004/)

Reason for revert:
[Sheriff] Breaks layout tests. Please submit a needsmanualrebaseline change on the blink side for the expectations if intended.

E.g.
http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/545

Original issue's description:
> Ship arrow functions
>
> R=rossberg@chromium.org
> LOG=Y
> BUG= v8:2700 
>
> Committed: https://crrev.com/541b6c39e0ecae1c070f51fae8e9e3dab18d278c
> Cr-Commit-Position: refs/heads/master@{#29119}

TBR=rossberg@chromium.org,wingo@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= v8:2700 

Review URL: https://codereview.chromium.org/1182053013

Cr-Commit-Position: refs/heads/master@{#29125}

[modify] http://crrev.com/bf92b53ff6724d6987dda643a9263135918fe051/src/flag-definitions.h

Comment 48 by wi...@igalia.com, Jun 19 2015

Blockedon: v8:4194

Comment 49 by wingo@chromium.org, Jun 19 2015

Blockedon: chromium:502243
Project Member

Comment 50 by bugdroid1@chromium.org, Jun 19 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9594cc72eec3ad64506c531106dba6c8fc42bae9

commit 9594cc72eec3ad64506c531106dba6c8fc42bae9
Author: wingo <wingo@igalia.com>
Date: Fri Jun 19 19:55:06 2015

Ship arrow functions

R=rossberg@chromium.org
LOG=Y
BUG= v8:2700 

Review URL: https://codereview.chromium.org/1194873002

Cr-Commit-Position: refs/heads/master@{#29167}

[modify] http://crrev.com/9594cc72eec3ad64506c531106dba6c8fc42bae9/src/flag-definitions.h

This feature is shipped. Can this issue be closed as 'Fixed'?
Status: Fixed
This issue should not be marked as `Fixed` until it's available in a stable version. I've tested on Chrome 45Beta and the lexical `this` feature are not supported.
@ #53:  What have you tried? The feature should work in Chrome 45. If not that would be a bug.

Regardless, please keep in mind that this is the V8 bug tracker, and it tracks the status of the V8 repo. When Chrome picks that up (or node.js, or any other embedder, for that matter) is a separate matter.
Arrow functions are working flawlessly for me on nightly by the way.

Comment 56 Deleted

@ross: I tried this code.
class Animal{
    cat(){
        new Sea().water(() => console.log(typeof this.water));
        /*
            * Result of `console.log(typeof this.water)`

            * V8 4.4.63.26, OS Ubuntu (iojs)
            * 'function'

            * V8 4.5.103.14, OS Windows (Chrome 45)
            * 'undefined'
        */
    }
}
class Sea{
    water(cb){
        cb.call(this);
    }
}
new Animal().cat();

The result shows that lexical `this` feature works on `V8 4.4` but `V8 4.5`. So which V8 version is this bug correctly fixed?
@lewis, #57: that is working as intended.  Lexical `this` means that it is bound to the `this` from the enclosing scope.  It cannot be retargeted using `call()` or `apply()`.  If you try the same code in Edge, Firefox or even the Babel transpiler¹, you'll see the same result.

¹ https://babeljs.io/repl/
@andy: I feel really embarassed right now. Thanks for pointing it out.
Project Member

Comment 60 by bugdroid1@chromium.org, Sep 2 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/510baeacbab311798d5e8795800ff773d00d062c

commit 510baeacbab311798d5e8795800ff773d00d062c
Author: caitpotter88 <caitpotter88@gmail.com>
Date: Wed Sep 02 21:10:51 2015

[es6] Re-implement rest parameters via desugaring.

Kills the kRestParameter bailout/disabled optimization, and fixes
lazily parsed arrow functions with rest parameters.

Supercedes https://crrev.com/1235153006/

BUG= chromium:508074 ,  v8:2160 ,  v8:2700 
LOG=N
R=rossberg@chromium.org, adamk@chromium.org, wingo@igalia.com

Review URL: https://codereview.chromium.org/1272673003

Cr-Commit-Position: refs/heads/master@{#30550}

[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/arm/code-stubs-arm.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/arm64/code-stubs-arm64.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/ast-literal-reindexer.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/ast-value-factory.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/bailout-reason.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/code-stubs.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/code-stubs.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/compiler/ast-graph-builder.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/compiler/ast-graph-builder.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/compiler/linkage.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/arm/full-codegen-arm.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/arm64/full-codegen-arm64.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/ia32/full-codegen-ia32.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/mips/full-codegen-mips.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/mips64/full-codegen-mips64.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/full-codegen/x64/full-codegen-x64.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/hydrogen.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/ia32/code-stubs-ia32.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/mips/code-stubs-mips.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/mips64/code-stubs-mips64.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/parser.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/parser.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/preparser.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/runtime/runtime-scopes.cc
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/runtime/runtime.h
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/src/x64/code-stubs-x64.cc
[add] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js
[add] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/test/mjsunit/harmony/regress/regress-508074.js
[modify] http://crrev.com/510baeacbab311798d5e8795800ff773d00d062c/test/mjsunit/harmony/rest-params.js

Project Member

Comment 61 by bugdroid1@chromium.org, Sep 3 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9628d86085ddee83067c45046ed89e0e6f635d3f

commit 9628d86085ddee83067c45046ed89e0e6f635d3f
Author: mbrandy <mbrandy@us.ibm.com>
Date: Thu Sep 03 18:40:09 2015

PPC: [es6] Re-implement rest parameters via desugaring.

Port 510baeacbab311798d5e8795800ff773d00d062c

Original commit message:
    Kills the kRestParameter bailout/disabled optimization, and fixes
    lazily parsed arrow functions with rest parameters.

    Supercedes https://crrev.com/1235153006/

R=caitpotter88@gmail.com, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com, dstence@us.ibm.com
BUG= chromium:508074 ,  v8:2160 ,  v8:2700 
LOG=N

Review URL: https://codereview.chromium.org/1318523006

Cr-Commit-Position: refs/heads/master@{#30574}

[modify] http://crrev.com/9628d86085ddee83067c45046ed89e0e6f635d3f/src/full-codegen/ppc/full-codegen-ppc.cc
[modify] http://crrev.com/9628d86085ddee83067c45046ed89e0e6f635d3f/src/ppc/code-stubs-ppc.cc

Not sure if this was mentioned, but arrow function implicit return doesn't seem work for new json objects:

x = t => { a: "b" };
x() -> returns undefined (expecting: {a:"b"} instance)

Happens on Version 45.0.2454.99 m (64-bit)
@ #62: you got buggy code. You meant to say:

x = t => ({a: "b"})

The meaning of your version is a block containing an expression statement "b" with a statement label a.
@danelK, I suspect that's intended behavior - curly braces are used to specify function block (that should actually be a SyntaxError)

I don't think that's an error; you may be unaware that "X:" is actually a valid Javascript language feature that allows to give a label to a statement[1]. So when you type: () => { a: "b" }, it means that you create a function that contains a single statement (a raw string expression "b"), labeled as `a`.

If you want to return a JSON object, you have to explicit that the function body is actually an expression, and wrap it between `()`. Ie. () => ({ a: "b" })

Thanks for the answers, but there's still something that I don't understand:
According to:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/label

The syntax for a label is:
label :   statement

and a string value is an expression and not a statement, ergo, this can't be a valid code block.

Another example may be:
function () {a:"b"}

This code throws an error:
Uncaught SyntaxError: Unexpected token ((…)

I also think that this is more intuitive, but I guess that this is besides the point :-)

What do you think?

Please don't use this bug report for Javascript tech support. Every time you post, 259 of us get an email notification, and you are just asking general questions about the Javascript language. FWIW, an expression is a type of statement, such as the string "use strict"; used as a statement to indicate strict mode in a Javascript engine.
Labels: Priority-1

Sign in to add a comment