Split AccessorInfo into DeclaredAccessorInfo and ExecutableAccessorInfo

Split AccessorInfo into DeclaredAccessorInfo and ExecutableAccessorInfo

R=svenpanne@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=13649

[dcarney, 2013-02-05 14:57:43]

I added a brief outline of the idea at the end of the v8 cheat sheet, which i'll
extend as things as flushed out.




In v8.h, I want to add the following, which is the api side of the
DeclaredAccessorDescriptor object.  The api will take a v8::AccessorDescriptor,
which will be transformed to a DeclaredAccessorDescriptor internally and used
for generate the hydrogen instructions.  DeclaredAccessorDescriptor is a
separate object as it must be able to contain a pointer to a child
DeclaredAccessorDescriptor.


enum AccessorDescriptorType {
  HANDLE_DESCRIPTOR,
  POINTER_COMPARE,
  BITMASK_COMPARE
};


struct BitmaskCompareDescriptor {
  uint8_t size;
  uint32_t bitmask;
  uint32_t compare_value;
};


struct PointerCompareDescriptor {
  intptr_t compare_value;
};


struct AccessorDescriptor {
  uint8_t internal_field;
  int16_t byte_offset;
  uint8_t accessor_descriptor_type;
  union {
    struct BitmaskCompareDescriptor bitmask_compare_descriptor;
    struct PointerCompareDescriptor pointer_compare_descriptor;
    struct AccessorDescriptor* handle_descriptor;
  };
};

[Sven Panne, 2013-02-07 10:01:03]

First round of comments. It would be helpful to see the other CL(s) building on
this one to see where we are actually heading...

https://codereview.chromium.org/12213012/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/12213012/diff/1/include/v8.h#newcode4204
include/v8.h:4204: static const int kJSObjectType = 0xad;
This is a strong hint that tools/grokdump.py needs an update, too. See comments
there how to do this.

https://codereview.chromium.org/12213012/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/12213012/diff/1/src/log.cc#newcode1646
src/log.cc:1646: if (!obj->IsExecutableAccessorInfo()) continue;
Hmmm, I am not sure is ignoring DeclaredAccessorInfo here is the right thing to
do.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode339
src/objects.cc:339: if (obj->IsExecutableAccessorInfo()) {
I think the old code should stay as it is and instead the AccessorInfo super
class should get everything it needs to make it so.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode398
src/objects.cc:398: if (obj->IsExecutableAccessorInfo()) {
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode2270
src/objects.cc:2270: Object* callback = callbacks.get(i);
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode2291
src/objects.cc:2291: AccessorInfo* entry;
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode2546
src/objects.cc:2546: if (obj->IsExecutableAccessorInfo()) {
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/objects.cc#newcode4824
src/objects.cc:4824: if (obj->IsExecutableAccessorInfo()) {
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/property.h
File src/property.h (right):

https://codereview.chromium.org/12213012/diff/1/src/property.h#newcode321
src/property.h:321: return callback->IsExecutableAccessorInfo() ||
callback->IsForeign();
Shouldn't the DescriptorAccessorInfos be data properties, too?

https://codereview.chromium.org/12213012/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/12213012/diff/1/src/runtime.cc#newcode984
src/runtime.cc:984: AccessorInfo* info = 0;
Again, this should stay as it is.

https://codereview.chromium.org/12213012/diff/1/src/runtime.cc#newcode4118
src/runtime.cc:4118: if (callback->IsExecutableAccessorInfo() ||
See comment above.

https://codereview.chromium.org/12213012/diff/1/src/runtime.cc#newcode10156
src/runtime.cc:10156: if (structure->IsForeign() ||
See comment above.

[dcarney, 2013-02-07 13:40:39]

https://codereview.chromium.org/12213012/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/12213012/diff/1/src/log.cc#newcode1646
src/log.cc:1646: if (!obj->IsExecutableAccessorInfo()) continue;
On 2013/02/07 10:01:03, Sven Panne wrote:
> Hmmm, I am not sure is ignoring DeclaredAccessorInfo here is the right thing
to
> do.

I really have no idea what's going on here.  Maybe you can suggest a solution?
No native callback is actually happening here, but I'm happy to log some new
thing.

[Sven Panne, 2013-02-12 13:44:38]

lgtm