Add utility functions for creating and accessing Dart_CObject structures

Add utility functions for creating and accessing Dart_CObject structures

All allocation is done using Dart_ScopeAllocate, so no explicit feeing
is required. This also means that these utilities require an active
ApiZone.

R=ager@google.com

BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=4388

[Mads Ager (google), 2012-02-20 12:38:38]

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:271: // Statically allocated Dart_Cobject instances for
immutable
Dart_CObject?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:273: // these is depending on that the marking
internally in the
is depending on that -> depends on the fact that ?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:275: Dart_CObject CObject::api_null_ =
{Dart_CObject::kNull , { 0 } };
I would add space between '{' and Dart_CObject.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:126: bool IsFalse() {
Blank line to separate these two?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:130: void* operator new(size_t size) {
Could we make the new operator private? It seems to me that users should be
using the methods below to create CObjects?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:138: static Dart_CObject* New(Dart_CObject::Type type,
int additional_bytes = 0);
Can we make this one private as well? This doesn't seem like something a user
should use either?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:141: // kBigint,
Add a TODO style comment instead?

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:146: // kByteArray,
Ditto.

[Søren Gjesse, 2012-02-21 08:24:25]

PTAL

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:271: // Statically allocated Dart_Cobject instances for
immutable
On 2012/02/20 12:38:38, Mads Ager wrote:
> Dart_CObject?

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:273: // these is depending on that the marking
internally in the
On 2012/02/20 12:38:38, Mads Ager wrote:
> is depending on that -> depends on the fact that ?

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:275: Dart_CObject CObject::api_null_ =
{Dart_CObject::kNull , { 0 } };
On 2012/02/20 12:38:38, Mads Ager wrote:
> I would add space between '{' and Dart_CObject.

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:126: bool IsFalse() {
On 2012/02/20 12:38:38, Mads Ager wrote:
> Blank line to separate these two?

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:130: void* operator new(size_t size) {
On 2012/02/20 12:38:38, Mads Ager wrote:
> Could we make the new operator private? It seems to me that users should be
> using the methods below to create CObjects?

So I know it looks a bit confusing, but an instance of "class CObject" here is a
wrapper for a Dart_CObject pointer. The instance of "class CObject" only
provides easy access to Dart_CObject structures. So the NewXXX methods below all
returns Dart_CObject pointers, whereas a "new CObject" only creates a new
wrapper. The "class CObject" instance is like a handle to the raw object
Dart_CObject.

So the usage pattern is like this: 

Dart_CObject* raw_array = CObject:NewArray(10);
CObject array = new CObject(raw_array);
CObject answer = new CObject(CObject:NewInt32(42););

array->SetAt(2, answer);
ASSERT(array[2]->IsInt32());

// Then return array caller...

One can also just have wrappers stack allocated:

Dart_CObject* raw_array = CObject:NewArray(10);
CObject array(raw_array);
CObject answer(CObject:NewInt32(42););

array.SetAt(2, &answer);
ASSERT(array[2]->IsInt32());

// Then return array.AsApiObject() to caller...

I did start out with "class CObject" embedding a Dart_CObject member, but then
one would have to have both "class CObjectEmbedded" and "class CObjectRef" to
handle newly created Dart_CObject structures and Dart_CObject structures
received in callback. Also a "class CObjectEmbedded" embedding a Dart_CObject
member whould still have to dynamically allocate additional storage for e.g.
strings and arrays.

This might not be the most elegant approach - I would like to hide some of the
wrapping details, but I could not come up with anything better. Also without the
zone allocation of both the "class CObject" wrappers and the Dart_CObject "raw
objects" this would have been much more complicated. 

Also one should be aware that each call to the operator[] on a COBjectArray
creates a new CObject wrapper in the zone.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:138: static Dart_CObject* New(Dart_CObject::Type type,
int additional_bytes = 0);
On 2012/02/20 12:38:38, Mads Ager wrote:
> Can we make this one private as well? This doesn't seem like something a user
> should use either?

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:141: // kBigint,
On 2012/02/20 12:38:38, Mads Ager wrote:
> Add a TODO style comment instead?

Done.

http://codereview.chromium.org/9403012/diff/9001/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:146: // kByteArray,
On 2012/02/20 12:38:38, Mads Ager wrote:
> Ditto.

Done.

[Mads Ager (google), 2012-02-21 08:51:19]

LGTM

We can put this back as is. We should do a follow-up change to simplify.

http://codereview.chromium.org/9403012/diff/13001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9403012/diff/13001/runtime/bin/dartutils.h#new...
runtime/bin/dartutils.h:108: class CObject {
As discussed offline I think we should get rid of the actual wrapping here. We
could have just a collection of static helper methods here that directly return
the Dart_CObjects. Then we don't allocate the wrappers and we don't have to
explicitly call AsApiCObject.

[Søren Gjesse, 2012-02-21 09:07:22]

http://codereview.chromium.org/9403012/diff/13001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9403012/diff/13001/runtime/bin/dartutils.h#new...
runtime/bin/dartutils.h:108: class CObject {
On 2012/02/21 08:51:20, Mads Ager wrote:
> As discussed offline I think we should get rid of the actual wrapping here. We
> could have just a collection of static helper methods here that directly
return
> the Dart_CObjects. Then we don't allocate the wrappers and we don't have to
> explicitly call AsApiCObject.

Will do.