Summary: | [Cocoa] More preparation for ARC, focusing on WebKit and smart pointers | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||
Component: | WebKit Misc. | Assignee: | Darin Adler <darin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, benjamin, cdumez, cmarcelo, dbates, ews-watchlist, mitz, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 186385 | ||||||
Attachments: |
|
Description
Darin Adler
2018-06-05 12:14:52 PDT
Created attachment 341984 [details]
Patch
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. 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. Committed r232520: <https://trac.webkit.org/changeset/232520> 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. 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!? |