Review comments addressed, please take another look...
http://codereview.chromium.org/9716035/diff/1/src/api.cc File src/api.cc (right): http://codereview.chromium.org/9716035/diff/1/src/api.cc#newcode3069 src/api.cc:3069: i::JSObject::TransformToFastProperties(Utils::OpenHandle(this), 0); On 2012/03/21 12:33:08, Michael Starzinger wrote:
It would be nice not to have this code duplicated in api.cc and
runtime.cc, but
I have no suggestion how to make it cleaner ATM.
Well, C++'s abstraction capabilities are extremely restricted, you basically have to define a class for everything. In our concrete example, we would need a bracket-like structure (using Andreas' favorite strategy pattern ;-), keeping the previous "fastness" as the local state. Not complicated, but a kind of a sledge-hammer for this, so I'll leave this as-is (as discussed offline). http://codereview.chromium.org/9716035/diff/1/src/objects.cc File src/objects.cc (left): http://codereview.chromium.org/9716035/diff/1/src/objects.cc#oldcode4517 src/objects.cc:4517: bool convert_back_to_fast = HasFastProperties() && On 2012/03/21 12:33:08, Michael Starzinger wrote:
Here we only convert the object back to fast properties when it had
fast
properties before. With the current change we will always convert back
to fast
properties after defining an accessor. I am not sure this what we
want. This was an accidental change in semantics, I don't think we always want to convert to fast mode, e.g. in the case of objects used as hash maps. I'll add the right logic to the callers. Done. http://codereview.chromium.org/9716035/diff/1/src/objects.h File src/objects.h (right): http://codereview.chromium.org/9716035/diff/1/src/objects.h#newcode8054 src/objects.h:8054: Object* SafeGet(AccessorComponent component); On 2012/03/21 12:33:08, Michael Starzinger wrote:
Since there is no "unsafe" version anymore, can we just call this
GetComponent()
or Get()?
I think I'll go for GetComponent, sounds like the right counterpart for SetComponents. Done. http://codereview.chromium.org/9716035/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/9716035/diff/1/src/runtime.cc#newcode4354 src/runtime.cc:4354: JSObject::TransformToFastProperties(obj, 0); On 2012/03/21 12:33:08, Michael Starzinger wrote:
See first comment.
Done. http://codereview.chromium.org/9716035/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
