https://codereview.chromium.org/705663004/diff/1/src/contexts.cc
File src/contexts.cc (right):
https://codereview.chromium.org/705663004/diff/1/src/contexts.cc#newcode24
src/contexts.cc:24: FixedArray::CopySize(table, length + 1));
This should perhaps double the table size, pre-allocating some slots.
https://codereview.chromium.org/705663004/diff/1/src/contexts.cc#newcode147
src/contexts.cc:147: // Note: Fixed context slots are statically
allocated by the compiler.
Nit: why not move this comment next to the UNREACHABLE below?
https://codereview.chromium.org/705663004/diff/1/src/contexts.h
File src/contexts.h (right):
https://codereview.chromium.org/705663004/diff/1/src/contexts.h#newcode199
src/contexts.h:199: int context_index_;
Nit: public data fields don't have a trailing "_".
https://codereview.chromium.org/705663004/diff/1/src/contexts.h#newcode203
src/contexts.h:203: MaybeAssignedFlag maybe_assigned_flag_;
Do we actually need the two flags for the global context table?
https://codereview.chromium.org/705663004/diff/1/src/runtime/runtime-scopes.cc
File src/runtime/runtime-scopes.cc (left):
https://codereview.chromium.org/705663004/diff/1/src/runtime/runtime-scopes.cc#oldcode522
src/runtime/runtime-scopes.cc:522:
result->global_object()->set_global_context(*result);
Can't the global_context property on the global object be removed
altogether now?
https://codereview.chromium.org/705663004/diff/1/src/runtime/runtime-scopes.cc
File src/runtime/runtime-scopes.cc (right):
https://codereview.chromium.org/705663004/diff/1/src/runtime/runtime-scopes.cc#newcode518
src/runtime/runtime-scopes.cc:518: if (IsLexicalVariableMode(mode) ||
IsLexicalVariableMode(lookup.mode_)) {
Are non-lexical variables actually included in those?
As discussed off-line, there needs to be an additional check against
non-configurable properties on the global object.
In fact, I think you also need to implement the inverse check in
DeclareGlobals, to diagnose 'var' clashing with earlier 'let'.
https://codereview.chromium.org/705663004/diff/1/src/runtime/runtime-scopes.cc#newcode540
src/runtime/runtime-scopes.cc:540: if (FindNameClash(scope_info,
global_context_table, &clashed_name)) {
Now that the checking this is a runtime thing and we don't have the
global scope chain anymore, I wonder if there aren't various places
where special handling of global scope or global context objects can be
removed.
https://codereview.chromium.org/705663004/
--
--
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.
