LGTM.
http://codereview.chromium.org/52021/diff/1/3 File include/v8.h (right): http://codereview.chromium.org/52021/diff/1/3#newcode1147 Line 1147: * The Wrap function V8 will return the most optimal Value object wrapping the Extra newline before 'The Wrap...' http://codereview.chromium.org/52021/diff/1/3#newcode1148 Line 1148: * c++ void*. The type of the value is not guaranteed to be an External object C++ (capital C) http://codereview.chromium.org/52021/diff/1/3#newcode1156 Line 1156: static void* Unwrap(Value* obj); Why doesn't this take a Handle<Value>? http://codereview.chromium.org/52021/diff/1/4 File src/api.cc (right): http://codereview.chromium.org/52021/diff/1/4#newcode2459 Line 2459: static Local<External> External_New_impl(void* data) { What's up with all the underscores? We don't usually use those in function names that have uppercase chars? Do you want this to be 'inline' too? http://codereview.chromium.org/52021/diff/1/4#newcode2463 Line 2463: static void* External_Value_impl(i::Handle<i::Object> obj) { Ditto. http://codereview.chromium.org/52021/diff/1/4#newcode2492 Line 2492: return reinterpret_cast<void*>(i::Smi::cast(*obj)->value() << Consider computing the value << kAlignedxxx before the return and stuff it in a local variable to improve readability. http://codereview.chromium.org/52021/diff/1/2 File test/cctest/test-api.cc (right): http://codereview.chromium.org/52021/diff/1/2#newcode1350 Line 1350: char* char_ptr = reinterpret_cast<char*>(v8::External::Unwrap(*zero)); Why aren't we just passing the handles here? Why does the Unwrap interface take a Value*? Seems a bit weird to me. http://codereview.chromium.org/52021 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
