Still LGTM!
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? On 2009/06/02 18:14:46, antonm wrote: > On 2009/06/02 17:14:04, Mads Ager wrote: > > I'm not sure I understand this TODO. Could you elaborate (and potentially > file > > a bug report and use the number)? > Sure. The code below is pretty close to JSObject::LocalLookupRealNamedProperty, > but doesn't have the check IsJSGlobalProxy() (cf. with the method), so I wonder > if it should be here as well. > BTW, what do you think about it? Ah, I understand now, thanks. I don't think the shortcutting for the JSGlobalProxy matters much for the current use of interceptors in chromium. However, the shortcutting is easy to do, so it probably makes sense to add it. If we add the shortcutting for the JSGlobalProxy here, we have to make sure to add it to GetInterceptorPropertyWithLookupHint as well so we use the hint on the right object. It is fine to leave that for another changelist. 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) { On 2009/06/02 18:14:46, antonm wrote: > On 2009/06/02 17:14:04, Mads Ager wrote: > > Use one line per argument here (as in GetInterceptorPropertyWithLookupHint)? > The problem is that if formatted as method_name(arg1, > arg2 > it goes out of 80 limit. What would you say about this variant? In that case, I normally do: method_name( arg1, arg2, ...) http://codereview.chromium.org/118118 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
