Bug 186314 – [Cocoa] More preparation for ARC, focusing on WebKit and smart pointers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

[Cocoa] More preparation for ARC, focusing on WebKit and smart pointers

[Darin Adler]

Created attachment 341984 [details]
Patch

[Anders Carlsson]

Comment on attachment 341984 [details]
Patch

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

> Source/WTF/wtf/BlockPtr.h:100
> +#if defined(__OBJC__) && __has_feature(objc_arc)

__has_feature(objc_arc) always implies __OBJC__ being defined, so you don't need to check for __OBJC__ here (or anywhere else).

> Source/WTF/wtf/OSObjectPtr.h:120
> +        OSObjectPtr ptr = other;

I think you can use std::move(other) here - under ARC Objective-C pointers have implicit move constructors.

[Darin Adler]

Comment on attachment 341984 [details]
Patch

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

>> Source/WTF/wtf/BlockPtr.h:100
>> +#if defined(__OBJC__) && __has_feature(objc_arc)
> 
> __has_feature(objc_arc) always implies __OBJC__ being defined, so you don't need to check for __OBJC__ here (or anywhere else).

That’s great. I will fix that everywhere. I had copied the #if from RetainPtr. I think I’ll use a separate patch for that.

>> Source/WTF/wtf/OSObjectPtr.h:120
>> +        OSObjectPtr ptr = other;
> 
> I think you can use std::move(other) here - under ARC Objective-C pointers have implicit move constructors.

That’s great news. Given that, maybe I should do a version that takes T&& too? For now I won’t do either of those things, but I will come back and do it as an optimization. Maybe with a unit test.

[Darin Adler]

Committed r232520: <https://trac.webkit.org/changeset/232520>

[Radar WebKit Bug Importer]

<rdar://problem/40822335>

Comment on attachment 341984 [details]
Patch

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

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:482
> -    IPC::encode(encoder, reinterpret_cast<CFDataRef>(archiver.get().encodedData));
> +    IPC::encode(encoder, (__bridge CFArrayRef)archiver.get().encodedData);

OOPS! This caused bug 186385.

[Darin Adler]

Comment on attachment 341984 [details]
Patch

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

>> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:482
>> +    IPC::encode(encoder, (__bridge CFArrayRef)archiver.get().encodedData);
> 
> OOPS! This caused bug 186385.

Darn, why didn’t I spot that!?