Addressed the comments and rebased. I do not quite like what I have done to
ic.cc, ideas on how to make it less ugly are welcome.
http://codereview.chromium.org/3047027/diff/16001/17003
File src/heap.cc (right):
http://codereview.chromium.org/3047027/diff/16001/17003#newcode328
src/heap.cc:328: ASSERT(gc_allowed_);
Cannot see much difference.
On 2010/07/30 17:18:57, antonm wrote:
maybe at the very start of the method?
http://codereview.chromium.org/3047027/diff/16001/17006
File src/ic.cc (right):
http://codereview.chromium.org/3047027/diff/16001/17006#newcode574
src/ic.cc:574: AssertNoGC nogc; // GC could invalidate the pointers
held in lookup.
I have done what I could. Cannot say I like what I see:)
On 2010/07/30 16:59:53, Vitaly wrote:
The comment implies that the nogc object should live as long as the
lookup
object. Can we introduce local scopes containing both these objects in
the
functions that call UpdateCaches?
http://codereview.chromium.org/3047027/diff/16001/17006#newcode574
src/ic.cc:574: AssertNoGC nogc; // GC could invalidate the pointers
held in lookup.
On 2010/07/30 17:18:57, antonm wrote:
I agree with Vitaly here (if I understand him correctly): why not at
the very
start of the function? Same for all other functions.
Done.
http://codereview.chromium.org/3047027/diff/16001/17006#newcode1483
src/ic.cc:1483: AssertNoGC nogc; // GC could invalidate the pointers
held in lookup.
On 2010/07/30 17:18:57, antonm wrote:
why not change ::Load/Store methods as well like:
LookupResult lookup;
{
AssertNoGc nogc;
LookupForRead(...);
...
}
PropertyAttributes attr;
Object* result = object->GetProperty(...)
?
That would allow us to catch additional problems.
Done.
http://codereview.chromium.org/3047027/diff/16001/17008
File src/stub-cache.h (right):
http://codereview.chromium.org/3047027/diff/16001/17008#newcode688
src/stub-cache.h:688: // exist and cannot be created without causing GC.
On 2010/07/30 17:18:57, antonm wrote:
nit: maybe don't overspecify: returns Failure if jump to miss stub
cannot be
generated?
by no means insisting.
Done.
http://codereview.chromium.org/3047027/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
