Kasper, Mads, tnx a lot for review! Submitting...
yours, anton. On Wed, Jun 3, 2009 at 9:56 AM, <[email protected]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
