LGTM if comments are addressed.
https://codereview.chromium.org/11412007/diff/2001/src/factory.cc File src/factory.cc (right): https://codereview.chromium.org/11412007/diff/2001/src/factory.cc#newcode1422 src/factory.cc:1422: Handle<Object> instance_template = Handle<Object>(desc->instance_template(), Use the handle constructor, that's easier to read I think. Handle<Object> instance_template(desc->instance_template(), isolate()); https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc File src/hydrogen.cc (right): https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc#newcode3240 src/hydrogen.cc:3240: Handle<Object> maybe_type_info(unoptimized_code->type_feedback_info(), I know this isn't your change, it must be something pretty recent. But I really don't like this maybe-handle pattern. The following is much easier to read and doesn't need an explicit isolate parameter. Handle<TypeFeedbackInfo> type_info( TypeFeedbackInfo::cast(unoptimized_code->type_feedback_info())); https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc#newcode7119 src/hydrogen.cc:7119: Handle<Object> maybe_type_info(unoptimized_code->type_feedback_info(), Likewise. https://codereview.chromium.org/11412007/diff/2001/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/11412007/diff/2001/src/runtime.cc#newcode3156 src/runtime.cc:3156: Handle<String> empty_string(isolate->heap()->empty_string()); Just use "Handle<String> empty_string = isolate->factory()->empty_string();" here, saves the handle allocation completely. https://codereview.chromium.org/11412007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
