LGTM when the following comments have been addressed. This is good stuff.
Also, you should implement this for ARM as well at which point I guess one of the GenerateLoadInterceptor functions can be removed? http://codereview.chromium.org/118118/diff/1/4 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/118118/diff/1/4#newcode363 Line 363: // TODO(367): maybe don't push lookup_hint for LOOKUP_IN_HOLDER and/or We normally capitalize the start of these TODO(xxx): messages: 'maybe' -> 'Maybe'. http://codereview.chromium.org/118118/diff/1/4#newcode1001 Line 1001: // TODO(368): compile in the whole chain: all the interceptors in Capitalize for consistency with other TODOs: 'compile' -> 'Compile'. http://codereview.chromium.org/118118/diff/1/7 File src/objects-inl.h (right): http://codereview.chromium.org/118118/diff/1/7#newcode2558 Line 2558: // TODO(antonm): do we want to do any shortcuts for global object? I'm not sure I understand this TODO. Could you elaborate (and potentially file a bug report and use the number)? http://codereview.chromium.org/118118/diff/1/3 File src/objects.cc (right): http://codereview.chromium.org/118118/diff/1/3#newcode5621 Line 5621: JSObject* receiver, String* name, PropertyAttributes* attributes) { Use one line per argument here (as in GetInterceptorPropertyWithLookupHint)? http://codereview.chromium.org/118118/diff/1/3#newcode5664 Line 5664: RETURN_IF_SCHEDULED_EXCEPTION(); I'm not sure this one is needed? I think that GetPropertyWithInterceptorProper handles scheduled exceptions by returning Failure::Exception. In that case, you should just propatate that out which is done by the return in the next line. http://codereview.chromium.org/118118/diff/1/3#newcode5676 Line 5676: RETURN_IF_SCHEDULED_EXCEPTION(); Is this one needed? What here can cause an exception to be scheduled? http://codereview.chromium.org/118118/diff/1/3#newcode5702 Line 5702: JSObject* receiver, String* name, PropertyAttributes* attributes) { One line per argument? http://codereview.chromium.org/118118/diff/1/3#newcode5709 Line 5709: RETURN_IF_SCHEDULED_EXCEPTION(); Is this one needed or is it handled by GetPropertyWithInterceptorProper already? http://codereview.chromium.org/118118/diff/1/6 File src/objects.h (right): http://codereview.chromium.org/118118/diff/1/6#newcode1354 Line 1354: Smi* payload, 'payload' -> 'hint'? http://codereview.chromium.org/118118/diff/1/6#newcode1357 Line 1357: enum { LOOKUP_IN_HOLDER = -1, LOOKUP_IN_PROTOTYPE = -2 }; For consistency with the rest of the code I would add these as constants to the list of constants later in the class definition (kMaxGap and friends). static const int kLookupInHolderHint = -1; static const int kLookupInPrototypeHint = -2; http://codereview.chromium.org/118118/diff/1/8 File src/stub-cache.h (right): http://codereview.chromium.org/118118/diff/1/8#newcode363 Line 363: Smi* lookup_payload, lookup_payload -> lookup_hint? http://codereview.chromium.org/118118 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
