[webkit-reviews] review granted: [Bug 4292] rewrite property lookup to make property slots explicit : [Attachment 3230] here it is

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Fri Aug 5 09:39:01 PDT 2005


Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 4292: rewrite property lookup to make property slots explicit
http://bugzilla.opendarwin.org/show_bug.cgi?id=4292

Attachment 3230: here it is
http://bugzilla.opendarwin.org/attachment.cgi?id=3230&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I think that the constant (GetValueFunc)1 should be a named constant rather
than an expression that's repeated over and over again.

bool isSet() should be a const member function.

Why do value slots need the slot base?

In "protection.h", why is the "object.h" include moved down? Isn't this
resolved another way using "object_wrapper.h".

You put the arguments prototype name and the prototype property name into the
Interpreter object. That seems like an unrelated optimization. Why include it
in the same patch?

I really wish you had defined the property slots in terms of ValueImp * rather
than Value -- it would have saved me a lot of work!

I noticed a typo: "seucirty check".

Otherwise, looks great.



More information about the webkit-reviews mailing list