This is now ready for review. I believe this is semantically correct and
does
not degrade performance when no let declarations are present.
Subsequent CLs:
- LoadIC and StoreIC handlers for global context loads.
- Cleanup of global context/global scope processing
- Cleanup of compilation cache
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));
On 2014/11/06 12:29:12, rossberg wrote:
This should perhaps double the table size, pre-allocating some slots.
Done.
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_;
On 2014/11/06 12:29:12, rossberg wrote:
Nit: public data fields don't have a trailing "_".
Done.
https://codereview.chromium.org/705663004/diff/1/src/contexts.h#newcode203
src/contexts.h:203: MaybeAssignedFlag maybe_assigned_flag_;
On 2014/11/06 12:29:12, rossberg wrote:
Do we actually need the two flags for the global context table?
They are used in Context::Lookup.
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);
On 2014/11/06 12:29:12, rossberg wrote:
Can't the global_context property on the global object be removed
altogether
now?
Yes, I will do the cleanup in separate CL.
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_)) {
On 2014/11/06 12:29:12, rossberg wrote:
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'.
All done (I kept IsLexicalVariableMode(lookup.mode) check for conistency
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)) {
On 2014/11/06 12:29:12, rossberg wrote:
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.
Yes, I will clean it up in separate CL
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.
