Added some comments.
https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc File src/objects.cc (right): https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3562 src/objects.cc:3562: MaybeObject* hidden_lookup = GetHiddenPropertiesHashTable(false); Can we make an enum out of the bool argument? https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3575 src/objects.cc:3575: if (hidden_lookup->ToObjectUnchecked()->IsUndefined()) { Replace hidden_lookup->ToObjectUnchecked() by inline_value. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3580 src/objects.cc:3580: ObjectHashTable::cast(hidden_lookup->ToObjectUnchecked()); Replace hidden_lookup->ToObjectUnchecked() by inline_value. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3581 src/objects.cc:3581: Object* entry = hashtable->Lookup(key); The API for hidden values does not convert strings as keys for hidden values to symbols. The StringDictionary handles normal strings in addition to symbols, while ObjectHashTable does not. We should probably convert all keys from the API to symbols to ensure we don't break the API. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3620 src/objects.cc:3620: if (!hidden_lookup->To<ObjectHashTable>(&hashtable)) return hidden_lookup; You don't need the <ObjectHashTable> afaik. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3625 src/objects.cc:3625: if (!insert_result->To<ObjectHashTable>(&new_table)) return insert_result; Don't need the <ObjectHashTable>. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3700 src/objects.cc:3700: } Please remove the { ... } around the MaybeObject. That's unnecessary; and we're slowly removing it everywhere. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3708 src/objects.cc:3708: if (!insert_result->To<ObjectHashTable>(&new_table)) return insert_result; Remove the <ObjectHashTable> https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode3725 src/objects.cc:3725: Object* hidden) { Can we rename hidden to hidden_properties or so? Put it on one line if it fits. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode11069 src/objects.cc:11069: bool force_capacity, Please use an enum. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode11182 src/objects.cc:11182: Allocate(nof * 2, false, pretenure ? TENURED : NOT_TENURED); enum. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.cc#newcode11211 src/objects.cc:11211: Allocate(at_least_room_for, false, pretenure ? TENURED : NOT_TENURED); enum. https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.h File src/objects.h (right): https://chromiumcodereview.appspot.com/10827040/diff/2009/src/objects.h#newcode2802 src/objects.h:2802: bool force_capacity = false, Can this be an enum? https://chromiumcodereview.appspot.com/10827040/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
