Thanks for the tips! I went back and forth a bit about renaming the locals in
the unit tests as well but felt that they were clearer as-is.


http://codereview.chromium.org/7344013/diff/1/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/7344013/diff/1/include/v8.h#newcode937
include/v8.h:937: V8EXPORT bool IsBoxedBoolean() const;
On 2011/07/12 20:38:12, Vyacheslav Egorov wrote:
Lets call them IsXXXObject instead of IsBoxedXXX.

Done.

http://codereview.chromium.org/7344013/diff/1/include/v8.h#newcode1778
include/v8.h:1778: class BoxedNumber : public Object {
On 2011/07/12 20:38:12, Vyacheslav Egorov wrote:
XXXObject

Done.

http://codereview.chromium.org/7344013/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/7344013/diff/1/src/api.cc#newcode2228
src/api.cc:2228: bool Value::IsMath() const {
On 2011/07/12 20:38:12, Vyacheslav Egorov wrote:
Lets pull IsMath() from API for now.

Done.

http://codereview.chromium.org/7344013/diff/1/src/factory.cc
File src/factory.cc (right):

http://codereview.chromium.org/7344013/diff/1/src/factory.cc#newcode249
src/factory.cc:249: isolate->heap()->AllocateJSObject(constructor),
On 2011/07/12 20:38:12, Vyacheslav Egorov wrote:
constructor will be "dead variable" if GC happens.

you need to wrap constructor into Handle.

(constructor object can't die, but potentially can be relocated)

better still: there is already a Object::ToObject and
Factory::ToObject methods
which do exactly what you need.

So I think you can use them to implement your API. so
Factory::NewBoxedXXX are
unnecessary.

Done.

http://codereview.chromium.org/7344013/

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

Reply via email to