Looking good, some comments before you write the other backends


https://codereview.chromium.org/791603003/diff/40001/src/ast-numbering.cc
File src/ast-numbering.cc (right):

https://codereview.chromium.org/791603003/diff/40001/src/ast-numbering.cc#newcode562
src/ast-numbering.cc:562: // TODO(aperez): Add support in Crankshaft for
arrow functions.
Let's file a bug for the tasks needed in Crankshaft, and mention the bug
here.

https://codereview.chromium.org/791603003/diff/40001/src/bailout-reason.h
File src/bailout-reason.h (right):

https://codereview.chromium.org/791603003/diff/40001/src/bailout-reason.h#newcode23
src/bailout-reason.h:23: V(kCapturedThis, "Receiver is captured in the
context")                      \
Please place in alphabetical order.  Tx :)

https://codereview.chromium.org/791603003/diff/40001/src/scopes.cc
File src/scopes.cc (right):

https://codereview.chromium.org/791603003/diff/40001/src/scopes.cc#newcode612
src/scopes.cc:612: // Arrow functions can cause the receiver to be
collected in the context.
"allocated".  And really it's "copied to" the context -- the receiver is
still in the stack slot from the outer function's perspective.

https://codereview.chromium.org/791603003/diff/40001/src/scopes.cc#newcode1206
src/scopes.cc:1206: if (inner->scope_uses_this_) {
Why do we need the whole list of these?  At some point this should be
refactored to be a bitfield, if this propagation is really needed.

https://codereview.chromium.org/791603003/diff/40001/src/scopes.cc#newcode1233
src/scopes.cc:1233: }
These propagations should run for all scopes

https://codereview.chromium.org/791603003/diff/40001/src/scopes.cc#newcode1361
src/scopes.cc:1361: // "this", it must be copied in the context, from
where it will be looked
s/it must be copied in/the receiver must be copied to/

https://codereview.chromium.org/791603003/diff/40001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/791603003/diff/40001/src/x64/full-codegen-x64.cc#newcode117
src/x64/full-codegen-x64.cc:117: // of the parent scope, so this does
not need to be done for those.
It's not necessarily the receiver from the parent scope -- the parent
scope could be a block scope, or another arrow function; anyway let's be
more precise with comments :)

https://codereview.chromium.org/791603003/diff/40001/src/x64/full-codegen-x64.cc#newcode225
src/x64/full-codegen-x64.cc:225: if (info->scope()->inner_uses_this()) {
Let's factor this into the loop below

https://codereview.chromium.org/791603003/diff/40001/src/x64/full-codegen-x64.cc#newcode2763
src/x64/full-codegen-x64.cc:2763: // is a sloppy mode method, or by the
function prologue it is an arrow
"if it is"

https://codereview.chromium.org/791603003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to