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
On 2009/03/23 18:21:03, Kasper Lund wrote:
> Extra newline before 'The Wrap...'

Done.

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
On 2009/03/23 18:21:03, Kasper Lund wrote:
> C++ (capital C)

"It was this way when I got here." I copied the style from above.
Changed all "c++" -> "C++" to be consistent.

http://codereview.chromium.org/52021/diff/1/3#newcode1156
Line 1156: static void* Unwrap(Value* obj);
On 2009/03/23 18:21:03, Kasper Lund wrote:
> Why doesn't this take a Handle<Value>?

Mainly because I mirrored the Cast API. Done.

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) {
On 2009/03/23 18:21:03, Kasper Lund wrote:
> 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?

Removed the underscores. I am leaving the inline decision up to the C++
compiler.

http://codereview.chromium.org/52021/diff/1/4#newcode2463
Line 2463: static void* External_Value_impl(i::Handle<i::Object> obj) {
On 2009/03/23 18:21:03, Kasper Lund wrote:
> Ditto.

Ditto.

http://codereview.chromium.org/52021/diff/1/4#newcode2492
Line 2492: return reinterpret_cast<void*>(i::Smi::cast(*obj)->value() <<
On 2009/03/23 18:21:03, Kasper Lund wrote:
> Consider computing the value << kAlignedxxx before the return and
stuff it in a
> local variable to improve readability.

Done.

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));
On 2009/03/23 18:21:03, Kasper Lund wrote:
> Why aren't we just passing the handles here? Why does the Unwrap
interface take
> a Value*? Seems a bit weird to me.

Done.

http://codereview.chromium.org/52021

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to