Thanks a lot for review, Kasper!
http://codereview.chromium.org/155141/diff/1003/1004 File src/ic.cc (right): http://codereview.chromium.org/155141/diff/1003/1004#newcode282 Line 282: if (lookup->IsNotFound() || lookup->type() != INTERCEPTOR || On 2009/07/08 06:55:06, Kasper Lund wrote: > Why did you add the !IsCacheable test here? Is it mostly a quick bailout check, > so we can avoid an expensive computation of the LookupResult when we know we'll > not really need it? I think it deserves a comment. Done. http://codereview.chromium.org/155141/diff/1003/1004#newcode293 Line 293: if (lookup->IsValid()) { On 2009/07/08 06:55:06, Kasper Lund wrote: > Do you want a non-cacheable bailout check here too? Well, I was somewhat uneasy here. If lookup is valid, we'll quit anyway, however if it's invalid and for some reason (that shouldn't happen now to the best of my knowledge) is not cacheable we would wrongly inform caller that lookup failed while it might be satisfied down the prototype chain. http://codereview.chromium.org/155141 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
