Bug 192816

Summary: Use idiomatic const references for equality methods in JSC::JSValue class
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: JavaScriptCoreAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED WONTFIX    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, ysuzuki
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=192813
Attachments:
Description Flags
Patch v1
ews-watchlist: commit-queue-
Patch v1 (retry EWS) none

David Kilzer (:ddkilzer)
Reported 2018-12-18 11:18:30 PST
Use idiomatic const references for equality helper methods for JSC::JSValue class.
Attachments
Patch v1 (5.88 KB, patch)
2018-12-18 11:26 PST, David Kilzer (:ddkilzer) 		ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Patch v1 (retry EWS) (5.88 KB, patch)
2018-12-20 04:51 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2018-12-18 11:26:51 PST
Created attachment 357588 [details] Patch v1
EWS Watchlist
Comment 2 2018-12-18 13:08:08 PST
Comment on attachment 357588 [details] Patch v1 Attachment 357588 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/10461961 New failing tests: stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no-cjit stress/const-semantics.js.ftl-eager-no-cjit apiTests
David Kilzer (:ddkilzer)
Comment 3 2018-12-18 14:10:56 PST
(In reply to Build Bot from comment #2) > Comment on attachment 357588 [details] > Patch v1 > > Attachment 357588 [details] did not pass jsc-ews (mac): > Output: https://webkit-queues.webkit.org/results/10461961 > > New failing tests: > stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no- > cjit > stress/const-semantics.js.ftl-eager-no-cjit > apiTests That's surprising! Will run the tests locally next.
David Kilzer (:ddkilzer)
Comment 4 2018-12-19 10:52:18 PST
(In reply to David Kilzer (:ddkilzer) from comment #3) > (In reply to Build Bot from comment #2) > > Comment on attachment 357588 [details] > > Patch v1 > > > > Attachment 357588 [details] did not pass jsc-ews (mac): > > Output: https://webkit-queues.webkit.org/results/10461961 > > > > New failing tests: > > stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no- > > cjit > > stress/const-semantics.js.ftl-eager-no-cjit > > apiTests > > That's surprising! Will run the tests locally next. Running locally I don't see the stress/* test failures: $ ./Tools/Scripts/run-javascriptcore-tests --no-build --no-fail-fast --release --verbose For apiTests, I see: 2018-12-19 10:50:22.980 testapi[83267:1353141] Testing Objective-C API 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Negative number maintained its original value": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "runJITThreadLimitTests() must run at the very beginning to test the case where the global JIT worklist was not initialized yet": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG threads should be the value provided through Options": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads should have been updated": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG threads should be the value provided through Options": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads should have been updated": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL threads should be the value provided through Options": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads should have been updated": FAILED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL threads should be the value provided through Options": PASSED 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads should have been updated": FAILED [...] FAIL: Some tests failed. testapi completed with rc=256 (1) Next I need to rerun the tests locally without the change.
David Kilzer (:ddkilzer)
Comment 5 2018-12-19 12:46:06 PST
(In reply to David Kilzer (:ddkilzer) from comment #4) > (In reply to David Kilzer (:ddkilzer) from comment #3) > > (In reply to Build Bot from comment #2) > > > Comment on attachment 357588 [details] > > > Patch v1 > > > > > > Attachment 357588 [details] did not pass jsc-ews (mac): > > > Output: https://webkit-queues.webkit.org/results/10461961 > > > > > > New failing tests: > > > stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no- > > > cjit > > > stress/const-semantics.js.ftl-eager-no-cjit > > > apiTests > > > > That's surprising! Will run the tests locally next. > > Running locally I don't see the stress/* test failures: > > $ ./Tools/Scripts/run-javascriptcore-tests --no-build --no-fail-fast > --release --verbose > > For apiTests, I see: > > 2018-12-19 10:50:22.980 testapi[83267:1353141] Testing Objective-C API > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Negative number > maintained its original value": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: > "runJITThreadLimitTests() must run at the very beginning to test the case > where the global JIT worklist was not initialized yet": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads > should have been updated": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads > should have been updated": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads > should have been updated": FAILED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads > should have been updated": FAILED > [...] > FAIL: Some tests failed. > testapi completed with rc=256 (1) > > Next I need to rerun the tests locally without the change. This was using a Release build of WebKit trunk r239276.
David Kilzer (:ddkilzer)
Comment 6 2018-12-20 04:50:27 PST
(In reply to David Kilzer (:ddkilzer) from comment #4) > (In reply to David Kilzer (:ddkilzer) from comment #3) > > (In reply to Build Bot from comment #2) > > > Comment on attachment 357588 [details] > > > Patch v1 > > > > > > Attachment 357588 [details] did not pass jsc-ews (mac): > > > Output: https://webkit-queues.webkit.org/results/10461961 > > > > > > New failing tests: > > > stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no- > > > cjit > > > stress/const-semantics.js.ftl-eager-no-cjit > > > apiTests > > > > That's surprising! Will run the tests locally next. > > Running locally I don't see the stress/* test failures: > > $ ./Tools/Scripts/run-javascriptcore-tests --no-build --no-fail-fast > --release --verbose > > For apiTests, I see: > > 2018-12-19 10:50:22.980 testapi[83267:1353141] Testing Objective-C API > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Negative number > maintained its original value": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: > "runJITThreadLimitTests() must run at the very beginning to test the case > where the global JIT worklist was not initialized yet": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads > should have been updated": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of DFG > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of DFG threads > should have been updated": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads > should have been updated": FAILED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Initial number of FTL > threads should be the value provided through Options": PASSED > 2018-12-19 10:50:22.988 testapi[83267:1353141] TEST: "Number of FTL threads > should have been updated": FAILED > [...] > FAIL: Some tests failed. > testapi completed with rc=256 (1) > > Next I need to rerun the tests locally without the change. The "Number of FTL threads should have been updated" test fails without this patch, so seems unrelated.
David Kilzer (:ddkilzer)
Comment 7 2018-12-20 04:51:26 PST
Created attachment 357806 [details] Patch v1 (retry EWS) Uploading Patch v1 (Attachment 357588 [details]) again to force a second EWS run.
David Kilzer (:ddkilzer)
Comment 8 2018-12-20 05:00:52 PST
Probably don't need to do this based on Darin's comment about WTF::StringView in Bug 192813 Comment #2.
Note You need to log in before you can comment on or make changes to this bug.