First round. A few nits and one real comment.
https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.cc File src/objects.cc (right): https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.cc#newcode4436 src/objects.cc:4436: MaybeObject* getterOk = GetHeap()->undefined_value(); Can we move the result of "GetHeap()" into a local "heap" variable? Same goes for the other methods below. https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.cc#newcode4649 src/objects.cc:4649: return NewCallbackTrans(name, component, accessor, attributes, accessors); Can we add an assertion to the top of this method that ASSERT(!accessor->IsMap()) to make sure we are not accidentally introducing transitions with this method. https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.cc#newcode4657 src/objects.cc:4657: MaybeObject* JSObject::CreateFreshAccessor(String* name, Can we turn this method into a static helper that returns a "MaybeMap" to transition to? The call to "set_map()" would be moved into the caller (i.e. "DefineFastAccessor"). That way all this complexity would be hidden from the API (i.e. JSObject) and there would only be one entry point to define fast JS-style accessors. https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.cc#newcode4709 src/objects.cc:4709: MaybeObject* JSObject::NewCallbackTrans(String* name, Likewise. Also the full name "NewCallbackTransition" would be better. https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.h File src/objects.h (right): https://chromiumcodereview.appspot.com/10238005/diff/1/src/objects.h#newcode713 src/objects.h:713: class AccessorPair; Can we alpha-sort this? https://chromiumcodereview.appspot.com/10238005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
