Thank you, Kevin. This way is much cleaner. I uploaded new patch set.
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()) {
On 2012/02/02 08:36:13, Kevin Millikin wrote:
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?
Indeed, this constructor is called only from DeserializeScopeChain.
https://chromiumcodereview.appspot.com/9310027/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
