Bug 221108 - check-webkit-style: warn about WTF::BlockPtr use in JavaScriptCore until ARC is enabled
Summary: check-webkit-style: warn about WTF::BlockPtr use in JavaScriptCore until ARC ...
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: 152799 186961
Blocks: 221117
  Show dependency treegraph
 
Reported: 2021-01-28 15:11 PST by David Kilzer (:ddkilzer)
Modified: 2021-01-29 10:23 PST (History)
13 users (show)

See Also:


Attachments
Patch v1 (10.34 KB, patch)
2021-01-28 20:27 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v2 (10.34 KB, patch)
2021-01-29 08:43 PST, David Kilzer (:ddkilzer)
ews-feeder: commit-queue-
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-01-28 15:11:09 PST
check-webkit-style: warn about WTF::BlockPtr use in JavaScriptCore until ARC is enabled.

This is to catch bugs like this from occurring before code is committed:

    Bug 220956: Crash when remote inspecting in debug builds
    <https://bugs.webkit.org/show_bug.cgi?id=220956>

See also:

    Bug 221107: JavaScriptCore should build with ARC enabled
    <https://bugs.webkit.org/show_bug.cgi?id=221107>
Comment 1 Radar WebKit Bug Importer 2021-01-28 15:11:40 PST
<rdar://problem/73726640>
Comment 2 David Kilzer (:ddkilzer) 2021-01-28 15:15:45 PST
This issue was introduced when WTF was converted to ARC since WTF::BlockPtr existed at that time:

    Bug 186961: [Cocoa] Convert the small bit of Objective-C++ code in WTF to ARC
    <https://bugs.webkit.org/show_bug.cgi?id=186961>

    Bug 152799: Add a smart block pointer
    <https://bugs.webkit.org/show_bug.cgi?id=152799>
Comment 3 David Kilzer (:ddkilzer) 2021-01-28 15:19:21 PST
(In reply to David Kilzer (:ddkilzer) from comment #2)
> This issue was introduced when WTF was converted to ARC since WTF::BlockPtr
> existed at that time:

Or rather, it became possible for this issue to exist once this happened, but we only know of one bug (Bug 220956) that was actually caused by this.
Comment 4 David Kilzer (:ddkilzer) 2021-01-28 20:27:36 PST
Created attachment 418702 [details]
Patch v1
Comment 5 David Kilzer (:ddkilzer) 2021-01-29 08:37:27 PST
(In reply to David Kilzer (:ddkilzer) from comment #4)
> Created attachment 418702 [details]
> Patch v1

Test failures are not related to this patch (missing results, or these *.xml files should be skipped since they're not tests.
Comment 6 David Kilzer (:ddkilzer) 2021-01-29 08:43:49 PST
Created attachment 418729 [details]
Patch v2
Comment 7 David Kilzer (:ddkilzer) 2021-01-29 08:44:41 PST
(In reply to David Kilzer (:ddkilzer) from comment #6)
> Created attachment 418729 [details]
> Patch v2

Made the regex check for "BlockPtr<" a little more robust.
Comment 8 Joseph Pecoraro 2021-01-29 10:02:01 PST
Comment on attachment 418729 [details]
Patch v2

Nice! r=me
Comment 9 David Kilzer (:ddkilzer) 2021-01-29 10:07:47 PST
The webkitpy failure is unrelated to this change (I think this Python test is flaky):

[41/1961] webkitcorepy.tests.task_pool_unittest.TaskPoolUnittest.test_invalid_shutdown failed:
  Traceback (most recent call last):
    File "/Volumes/Data/worker/WebKitPy-Tests-EWS/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/tests/task_pool_unittest.py", line 136, in test_invalid_shutdown
      pool.do(wait, 2)
  AssertionError: Exception not raised
Comment 10 David Kilzer (:ddkilzer) 2021-01-29 10:09:23 PST
(In reply to David Kilzer (:ddkilzer) from comment #9)
> The webkitpy failure is unrelated to this change (I think this Python test
> is flaky):
> 
> [41/1961]
> webkitcorepy.tests.task_pool_unittest.TaskPoolUnittest.test_invalid_shutdown
> failed:
>   Traceback (most recent call last):
>     File
> "/Volumes/Data/worker/WebKitPy-Tests-EWS/build/Tools/Scripts/libraries/
> webkitcorepy/webkitcorepy/tests/task_pool_unittest.py", line 136, in
> test_invalid_shutdown
>       pool.do(wait, 2)
>   AssertionError: Exception not raised

<https://ews-build.webkit.org/#/builders/5/builds/42925>
Comment 11 EWS 2021-01-29 10:11:52 PST
Committed r272064: <https://trac.webkit.org/changeset/272064>

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