All suggested changes made, except refactoring. Instead, scope of an if has been reduced dramatically in AddFastProperty, eliminating duplicate code. Tested & Linted. New changes to AddFastProperty uploaded to this changelist.
On 2008/10/08 10:38:25, bak wrote: > LGTM, > Lars > > http://codereview.chromium.org/6528/diff/1/2 > File src/objects.cc (right): > > http://codereview.chromium.org/6528/diff/1/2#newcode1223 > Line 1223: // Make new properties array if necessary. > Please check if you could refactor the code below. It is used > in other functions. > > http://codereview.chromium.org/6528/diff/1/2#newcode1552 > Line 1552: } else { > remove } else { }. > keep the return ConvertDescriptorToField(name, value, attributes); > > http://codereview.chromium.org/6528/diff/1/2#newcode1603 > Line 1603: /* > Please remove the dead code but leave a comment. > There are two other places in this function. > > http://codereview.chromium.org/6528/diff/1/2#newcode1661 > Line 1661: case NULL_DESCRIPTOR: > Code not used yet. I was a bit confused since it is never used. > > http://codereview.chromium.org/6528/diff/1/3 > File src/objects.h (right): > > http://codereview.chromium.org/6528/diff/1/3#newcode1395 > Line 1395: // When adding to the properties FixedArray, we increase its size by > When extending the backing storage for property values, ... http://codereview.chromium.org/6528 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
