Like it, mostly nits.
https://codereview.chromium.org/716833002/diff/1/src/full-codegen.cc
File src/full-codegen.cc (right):
https://codereview.chromium.org/716833002/diff/1/src/full-codegen.cc#newcode647
src/full-codegen.cc:647: // (innermost) enclosing script scope. To deal
with recursion, nested modules
Nit: can remove the "(innermost)"
https://codereview.chromium.org/716833002/diff/1/src/globals.h
File src/globals.h (right):
https://codereview.chromium.org/716833002/diff/1/src/globals.h#newcode654
src/globals.h:654: SCRIPT_SCOPE, // The top-level scope for a program
or a top-level eval.
Nit: s/program/script/
https://codereview.chromium.org/716833002/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/716833002/diff/1/src/parser.cc#newcode910
src/parser.cc:910: *scope =
Scope::DeserializeScopeChain(*info->context(), *scope, zone());
Might be worth having an assertion after this:
DCHECK(!(*scope)->is_script_scope());
https://codereview.chromium.org/716833002/diff/1/src/parser.cc#newcode1797
src/parser.cc:1797: // not a var (in the script scope, we also have to
ignore legacy const for
Nit: s/the script/a script/
https://codereview.chromium.org/716833002/diff/1/src/parser.cc#newcode1981
src/parser.cc:1981: // In ES6, a function behaves as a lexical binding,
except in the
Nit: s/except in the/except in a/
https://codereview.chromium.org/716833002/diff/1/src/runtime/runtime-debug.cc
File src/runtime/runtime-debug.cc (right):
https://codereview.chromium.org/716833002/diff/1/src/runtime/runtime-debug.cc#newcode1227
src/runtime/runtime-debug.cc:1227: DCHECK(context_->IsNativeContext());
Note: this is all wrong and needs fixing, but that's for a separate CL.
https://codereview.chromium.org/716833002/diff/1/src/runtime/runtime-scopes.cc
File src/runtime/runtime-scopes.cc (right):
https://codereview.chromium.org/716833002/diff/1/src/runtime/runtime-scopes.cc#newcode200
src/runtime/runtime-scopes.cc:200: // Declarations are always made in a
function, native, or script context. In
Hm, I wonder if this comment is accurate. I think it should drop
"native", but add "eval".
https://codereview.chromium.org/716833002/diff/1/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/716833002/diff/1/src/scopes.cc#newcode733
src/scopes.cc:733: while (!scope->is_script_scope()) {
This loop should no longer be needed. (Is this method even needed?)
https://codereview.chromium.org/716833002/diff/1/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/716833002/diff/1/src/scopes.h#newcode399
src/scopes.h:399: // Find the innermost script scope.
Nit: remove "innermost". Do we actually still need this method?
https://codereview.chromium.org/716833002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
