On Fri, Aug 20, 2010 at 12:01 PM, <[email protected]> wrote: > LGTM > > > http://codereview.chromium.org/3143032/diff/1/2 > File src/builtins.cc (right): > > http://codereview.chromium.org/3143032/diff/1/2#newcode332 > src/builtins.cc:332: return elms + to_trim * kPointerSize; > There is an appropriate assertion just above return. > > I do not understand why Anton decided to use such expression though.
I don't remember any particular reason for that, most probably that was just an oversight on my side. Thanks a lot for spotting and fixing it, guys. > > On 2010/08/20 00:06:46, Vitaly wrote: >> >> This line advances elms pointer by (to_trim * kPointerSize) fixed > > arrays, but >> >> since our object classes have no data members, sizeof(FixedArray) > > happens to be >> >> 1 (the least allowed size), and advancing by this many fixed arrays is >> equivalent to advancing by this many bytes. I think this is too subtle > > to rely >> >> on. > > http://codereview.chromium.org/3143032/diff/1/2#newcode725 > src/builtins.cc:725: const bool trim_array = > Heap::new_space()->Contains(elms) && > You should also fix this condition. > > http://codereview.chromium.org/3143032/show > > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
