There might be a simpler and safer way:
https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.cc File src/scopes.cc (left): https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.cc#oldcode152 src/scopes.cc:152: if (*scope_info != ScopeInfo::Empty()) { As a simple cleanup, you can get rid of this completely (SetDefaults already set language_mode for all scopes, no need to overwrite it---unless I'm reading the code wrong). https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.cc File src/scopes.cc (right): https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.cc#newcode151 src/scopes.cc:151: if (!scope_info.is_null() && *scope_info != ScopeInfo::Empty()) { As far as I can tell, there is no reason *at all* that this function should ever produce a scope indicating there's no context---because there is definitely a materialized context. I think it's much more straightforward to write this function as: SetDefaults(type, NULL, scope_info); if (!scope_info.is_null()) { num_heap_slots_ = scope_info->ContextLength(); } // Ensure at least MIN_CONTEXT_SLOTS to indicate a materialized context. num_heap_slots_ = Max(num_heap_slots_, Context::MIN_CONTEXT_SLOTS); AddInnerScope(inner_scope); And no need for the default-valued parameter. Does that implementation make sense? https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.h File src/scopes.h (right): https://chromiumcodereview.appspot.com/9310027/diff/1/src/scopes.h#newcode549 src/scopes.h:549: bool has_context_with_extension = false); Please no: default parameters are bad, boolean-valued default parameters are really bad. https://chromiumcodereview.appspot.com/9310027/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
