WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
195962
Remove copyRef() calls added in
r243163
https://bugs.webkit.org/show_bug.cgi?id=195962
Summary
Remove copyRef() calls added in r243163
Michael Catanzaro
Reported
2019-03-19 13:56:56 PDT
I added some calls to copyRef() in
r243163
that Chris wants removed, see
bug #195920
.
Attachments
Patch
(4.12 KB, patch)
2019-03-19 15:34 PDT
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews202 for win-future
(12.92 MB, application/zip)
2019-03-19 19:00 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(3.04 MB, application/zip)
2019-03-19 19:58 PDT
,
EWS Watchlist
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Michael Catanzaro
Comment 1
2019-03-19 15:34:06 PDT
Created
attachment 365245
[details]
Patch
EWS Watchlist
Comment 2
2019-03-19 15:36:56 PDT
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
EWS Watchlist
Comment 3
2019-03-19 19:00:31 PDT
Comment on
attachment 365245
[details]
Patch
Attachment 365245
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11573478
New failing tests: js/dom/custom-constructors.html
EWS Watchlist
Comment 4
2019-03-19 19:00:42 PDT
Created
attachment 365286
[details]
Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
EWS Watchlist
Comment 5
2019-03-19 19:58:35 PDT
Comment on
attachment 365245
[details]
Patch
Attachment 365245
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11574224
New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
EWS Watchlist
Comment 6
2019-03-19 19:58:37 PDT
Created
attachment 365297
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Chris Dumez
Comment 7
2019-03-20 09:13:43 PDT
Comment on
attachment 365245
[details]
Patch Clearing flags on attachment: 365245 Committed
r243204
: <
https://trac.webkit.org/changeset/243204
>
Chris Dumez
Comment 8
2019-03-20 09:13:45 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9
2019-03-20 09:14:46 PDT
<
rdar://problem/49063670
>
Darin Adler
Comment 10
2019-03-20 09:55:05 PDT
Comment on
attachment 365245
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=365245&action=review
> Source/JavaScriptCore/inspector/scripts/codegen/cpp_generator_templates.py:235 > + Ref<JSON::Object> jsonResult = m_result.releaseNonNull(); > + auto result = WTFMove(*reinterpret_cast<Ref<${objectType}>*>(&jsonResult)); > + return result;
I would think we’d need to add a comment explaining that this is to work around a GCC 9 bug. Otherwise, someone could refactor this to get rid of the local variable. Another way to protect against that is to make sure we have at least EWS testing completion with GCC 9.
Michael Catanzaro
Comment 11
2019-03-20 10:09:13 PDT
(In reply to Darin Adler from
comment #10
)
> I would think we’d need to add a comment explaining that this is to work > around a GCC 9 bug. Otherwise, someone could refactor this to get rid of the > local variable. Another way to protect against that is to make sure we have > at least EWS testing completion with GCC 9.
I almost added a comment, but in the end I decided it was low-value since if it's ever refactored, we'll either get a warning again and know right away, or else we won't (if GCC stops warning there) and it will be fine. EWS won't catch it unless we enable -Werror. There are pros and cons to enabling -Werror on the EWS. The risk is that it will probably significantly increase the number of build failures. The benefit is that it will surely reduce the number of compiler warnings introduced. (I agree it would be nice to have EWS testing more supported versions of GCC.)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug