Update the patch, fixed the CallAsFunction case, and added tests for
them.

Please take another look?

On Sep 7, 10:29 am, Feng <[EMAIL PROTECTED]> wrote:
> On Sep 7, 12:23 am, "Kasper Lund" <[EMAIL PROTECTED]> wrote:
>
> > LGTM, but you should extend your test case to try to call a
> > non-function and make sure you still get the proper exception (this is
> > a must). Ideally you should also add a test that shows that it works
> > for objects that has a function delegate to test-api.cc
> > (CallAsFunction).
>
> Good point, will add test cases before submitting.
> I am confident the first case should work as before (throwing
> exception on non-function object);
> The second case is likely broken, verifying..
>
> > Cheers,
> > Kasper
>
> > On Sun, Sep 7, 2008 at 6:47 AM,  <[EMAIL PROTECTED]> wrote:
> > > Reviewers: Kasper Lund,
>
> > > Description:
> > > Fix issuehttp://code.google.com/p/v8/issues/detail?id=32
>
> > > Allows numberical strings as array index and make a call. e.g.,
> > > callbacks['0']();
>
> > > Please review this athttp://codereview.chromium.org/1604
>
> > > Affected files:
> > >  M     src/ic.cc
> > >  A     test/mjsunit/number-string-index-call.js
>
> > > Index: test/mjsunit/number-string-index-call.js
> > > ===================================================================
> > > --- test/mjsunit/number-string-index-call.js    (revision 0)
> > > +++ test/mjsunit/number-string-index-call.js    (revision 0)
> > > @@ -0,0 +1,3 @@
> > > +var callbacks = [ function() {return 'foo'} ];
> > > +var result = callbacks['0']();
> > > +assertEquals('foo', result);
> > > Index: src/ic.cc
> > > ===================================================================
> > > --- src/ic.cc   (revision 184)
> > > +++ src/ic.cc   (working copy)
> > > @@ -259,12 +259,20 @@
> > >     return TypeError("non_object_property_call", object, name);
> > >   }
>
> > > +  Object* result = Heap::the_hole_value();
> > > +
> > > +  // Check if the name is trivially convertible to an index and get
> > > +  // the element if so.
> > > +  uint32_t index;
> > > +  if (name->AsArrayIndex(&index)) {
> > > +    result = object->GetElement(index);
> > > +    if (result->IsJSFunction()) return result;
> > > +  }
> > > +
> > >   // Lookup the property in the object.
> > >   LookupResult lookup;
> > >   object->Lookup(*name, &lookup);
>
> > > -  Object* result = Heap::the_hole_value();
> > > -
> > >   if (!lookup.IsValid()) {
> > >     // If the object does not have the requested property, check which
> > >     // exception we need to throw.
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to