Bug 53902 – JSVariableObject::setRegisters should take PassOwnArrayPtr for registersArray.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryosuke Niwa]

JSVariableObject::setRegisters currently takes two pointers to Register but registersArray should be PassOwnArrayPtr instead.

[Ryosuke Niwa]

Created attachment 81449 [details]
Patch

[Darin Adler]

Comment on attachment 81449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=81449&action=review

> Source/JavaScriptCore/runtime/JSVariableObject.h:162
> +        ASSERT(registerArray.get() != d->registerArray.get());

I’m surprised that either get() is required. We should make the smart pointers work for equality comparison without using the get().

[Ryosuke Niwa]

(In reply to comment #2)
> (From update of attachment 81449 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=81449&action=review
> 
> > Source/JavaScriptCore/runtime/JSVariableObject.h:162
> > +        ASSERT(registerArray.get() != d->registerArray.get());
> 
> I’m surprised that either get() is required. We should make the smart pointers work for equality comparison without using the get().

Oops, no.  That's just me being careless.  We don't need to call get manually.  Will fix before landing the patch.

[Ryosuke Niwa]

Comment on attachment 81449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=81449&action=review

> Source/JavaScriptCore/runtime/Arguments.h:251
> +        OwnArrayPtr<Register> registerArray = copyRegisterArray(d()->registers - registerOffset, registerArraySize);
> +        setRegisters(registerArray.get() + registerOffset, registerArray.release());

Oops, I need a temporary variable for this as well.  Will fix before landing.

[Darin Adler]

(In reply to comment #4)
> View in context: https://bugs.webkit.org/attachment.cgi?id=81449&action=review
> 
> > Source/JavaScriptCore/runtime/Arguments.h:251
> > +        OwnArrayPtr<Register> registerArray = copyRegisterArray(d()->registers - registerOffset, registerArraySize);
> > +        setRegisters(registerArray.get() + registerOffset, registerArray.release());
> 
> Oops, I need a temporary variable for this as well.  Will fix before landing.

Oh, can’t believe I missed that!