Bug 231251 - Add back RetainPtrARC and TypeCastsCocoaARC tests to TestWTF
Summary: Add back RetainPtrARC and TypeCastsCocoaARC tests to TestWTF
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on: 230406 231172
Blocks: 231467
  Show dependency treegraph
 
Reported: 2021-10-05 14:01 PDT by David Kilzer (:ddkilzer)
Modified: 2021-10-15 10:53 PDT (History)
7 users (show)

See Also:


Attachments
Patch v1 (28.34 KB, patch)
2021-10-06 16:44 PDT, David Kilzer (:ddkilzer)
darin: review+
Details | Formatted Diff | Diff
Patch for landing (27.17 KB, patch)
2021-10-06 18:37 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2021-10-05 14:01:18 PDT
Add back RetainPtrARC and TypeCastsCocoaARC tests to TestWTF.

They were removed for:

    Bug 230406: WTF::RetainPtr<> allows assignment of two pointer types that are not assignable

They were removed to make the bots green again:

    Bug 231172: REGRESSION (r283476): [ iOS macOS Debug ] TestWTF failures in RetainPtrARC and TypeCastsCocoaARC
Comment 1 Radar WebKit Bug Importer 2021-10-05 14:02:19 PDT
<rdar://problem/83902809>
Comment 3 David Kilzer (:ddkilzer) 2021-10-06 16:44:13 PDT
Created attachment 440455 [details]
Patch v1
Comment 4 Darin Adler 2021-10-06 18:24:02 PDT
Comment on attachment 440455 [details]
Patch v1

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

Nice solution.

> Source/WTF/ChangeLog:12
> +        * wtf/cocoa/TypeCastsCocoa.h:
> +        (checked_objc_castArc):
> +        - Change `checked_objc_castARC` to `checked_objc_castArc` to
> +          match `adoptNSArc`.

Nice to match, but not sure why Chris decided to turn ARC into a word.

> Tools/TestWebKitAPI/Tests/WTF/cocoa/TypeCastsCocoa.mm:38
> +#define TEST_WTF_BEGIN_AUTORELEASEPOOL @autoreleasepool {

I suggest a name like: BEGIN_AUTORELEASEPOOL_FOR_ARC_DEBUG

I understand the concept of using "WTF" to avoid conflicts, and "TEST", but I don’t think either are needed in practice.
Comment 5 David Kilzer (:ddkilzer) 2021-10-06 18:33:33 PDT
Comment on attachment 440455 [details]
Patch v1

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

>> Source/WTF/ChangeLog:12
>> +          match `adoptNSArc`.
> 
> Nice to match, but not sure why Chris decided to turn ARC into a word.

I'll revert this change for the final patch.

>> Tools/TestWebKitAPI/Tests/WTF/cocoa/TypeCastsCocoa.mm:38
>> +#define TEST_WTF_BEGIN_AUTORELEASEPOOL @autoreleasepool {
> 
> I suggest a name like: BEGIN_AUTORELEASEPOOL_FOR_ARC_DEBUG
> 
> I understand the concept of using "WTF" to avoid conflicts, and "TEST", but I don’t think either are needed in practice.

Will do.
Comment 6 David Kilzer (:ddkilzer) 2021-10-06 18:37:06 PDT
Created attachment 440462 [details]
Patch for landing
Comment 7 EWS 2021-10-07 08:02:31 PDT
Committed r283712 (242638@main): <https://commits.webkit.org/242638@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440462 [details].