[webkit-reviews] review denied: [Bug 55554] Remove LOOSE_PASS_OWN_ARRAY_PTR from PassOwnArrayPtr.h : [Attachment 84374] removed LOOSE_PASS_OWN_ARRAY_PTR

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 55554: Remove LOOSE_PASS_OWN_ARRAY_PTR from PassOwnArrayPtr.h
https://bugs.webkit.org/show_bug.cgi?id=55554

Attachment 84374: removed LOOSE_PASS_OWN_ARRAY_PTR
https://bugs.webkit.org/attachment.cgi?id=84374&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=84374&action=review

I’m thrilled that you tackled this!

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:2543
>			compatibilityVersion = "Xcode 3.1";
> +			developmentRegion = English;
>			hasScannedForEncodings = 1;

Darn it, has this started happening again? I can’t remember what causes this.

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431
>	   Register* registerArray = new Register[newSize];
>	   memcpy(registerArray + newSize - oldSize, d()->registerArray.get(),
oldSize * sizeof(Register));
> -	   setRegisters(registerArray + newSize, registerArray, newSize);
> +	   setRegisters(registerArray + newSize, adoptArrayPtr(registerArray),
newSize);

We want the adopt to be right after the "new", in the same expression if
possible. So please change the local variable to an OwnArrayPtr and add the
release and get calls as needed.

> Source/JavaScriptCore/runtime/JSGlobalObject.h:320
>	   Register* registerArray = new Register[newSize];
>	   if (d()->registerArray)
>	       memcpy(registerArray + count, d()->registerArray.get(), oldSize
* sizeof(Register));
> -	   setRegisters(registerArray + newSize, registerArray, newSize);
> +	   setRegisters(registerArray + newSize, adoptArrayPtr(registerArray),
newSize);

We want the adopt to be right after the "new", in the same expression if
possible. So please change the local variable to an OwnArrayPtr and add the
release and get calls as needed.

> Source/JavaScriptCore/wtf/PassOwnArrayPtr.h:-91
> -#if !defined(LOOSE_PASS_OWN_ARRAY_PTR) || !HAVE(NULLPTR)
>      PassOwnArrayPtr& operator=(std::nullptr_t) { clear(); return *this; }
> -#endif
> +
>      template<typename U> PassOwnArrayPtr& operator=(const
PassOwnArrayPtr<U>&);
>  
>      template<typename U> friend PassOwnArrayPtr<U> adoptArrayPtr(U*);
>  
> -#ifdef LOOSE_PASS_OWN_ARRAY_PTR
> -    PassOwnArrayPtr(PtrType ptr) : m_ptr(ptr) { }
> -    PassOwnArrayPtr& operator=(PtrType);
> -#endif
> -
>  private:
> -#ifndef LOOSE_PASS_OWN_ARRAY_PTR
>      explicit PassOwnArrayPtr(PtrType ptr) : m_ptr(ptr) { }
> -#endif

Please don’t remove the loose feature entirely; revert this part of the patch
at least for now. We are using this in some non-WebKit code, and for a while
we’ll need the loose mode there. It’s fine to turn the loose mode off, though.