Ready for round two. PTAL.
https://codereview.chromium.org/722793005/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/722793005/diff/1/src/ast.h#newcode2675
src/ast.h:2675: VariableProxy* proxy() const { return proxy_; }
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
Nit: rename proxy => class_variable_proxy (also field)
"Proxy" is way overloaded.
Done.
https://codereview.chromium.org/722793005/diff/1/src/ast.h#newcode2684
src/ast.h:2684: VariableProxy* proxy, Expression* extends,
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
proxy => class_variable_proxy
Done.
https://codereview.chromium.org/722793005/diff/1/src/ast.h#newcode2699
src/ast.h:2699: VariableProxy* proxy_;
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
proxy_ => class_variable_proxy_
Done.
https://codereview.chromium.org/722793005/diff/1/src/full-codegen.cc
File src/full-codegen.cc (right):
https://codereview.chromium.org/722793005/diff/1/src/full-codegen.cc#newcode1613
src/full-codegen.cc:1613: Comment cmnt(masm_, "[ Extend block context");
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
Please share code with VisitBlock here (add an EnterBlockScope method)
We will also need bailout ids here in the future, when/if we
hydrogenize/turbofanize this.
I suggest extracting that part too (so both context pushing and
visiting
declarations below are part of EnterBlockScope method), parameterizing
EnterBlockScope with BailoutIds and for now passing BailoutId::None
here.
Done.
https://codereview.chromium.org/722793005/diff/1/src/full-codegen.cc#newcode1651
src/full-codegen.cc:1651: LoadContextField(context_register(),
Context::PREVIOUS_INDEX);
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
Please share code with VisitBlock here (add an ExitBlockScope method)
Same as above for BailoutIds.
Done.
https://codereview.chromium.org/722793005/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/722793005/diff/1/src/parser.cc#newcode3932
src/parser.cc:3932: if (this->IsEvalOrArguments(name)) {
On 2014/11/13 01:45:43, adamk wrote:
I don't think you need these explicit |this| references now that your
code is in
{,pre}parser.cc
Same goes everywhere below where "this->" appears.
Done.
https://codereview.chromium.org/722793005/diff/1/src/parser.cc#newcode3976
src/parser.cc:3976: if (has_seen_constructor !=
old_has_seen_constructor) {
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
Nit: replace this condition with 'constructor == NULL &&
has_seen_constructor'
and remove old_has_seen_constructor.
Done.
https://codereview.chromium.org/722793005/diff/1/src/parser.cc#newcode3991
src/parser.cc:3991: if (!has_seen_constructor) {
On 2014/11/13 11:17:37, Dmitry Lomov (chromium) wrote:
Nit: replace this condition with 'constructor == NULL'
Done.
https://codereview.chromium.org/722793005/diff/1/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/722793005/diff/1/src/preparser.cc#newcode946
src/preparser.cc:946: return this->EmptyExpression();
On 2014/11/13 01:45:44, adamk wrote:
"this->" unnecessary here and below, as well.
Done.
https://codereview.chromium.org/722793005/
--
--
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.
