| Summary: | Fix -Wcast-qual and -Wunused-lambda-capture warnings in WebCore with new clang compiler | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||
| Component: | WebCore Misc. | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | achristensen, buildbot, cdumez, commit-queue, ddkilzer, eric.carlson, jer.noble, jiewen_tan, kling, koivisto, mmaxfield, rniwa, sam, simon.fraser, thorton, youennf, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=178036 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
David Kilzer (:ddkilzer)
2017-08-23 15:52:37 PDT
Created attachment 318933 [details]
Patch
Comment on attachment 318933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318933&action=review > Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:-139 > - (uint8_t*)keyData.modulus().data(), keyData.modulus().size(), In fact, I don't understand why (uint8_t*) is even needed initially. Comment on attachment 318933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318933&action=review >> Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:-139 >> - (uint8_t*)keyData.modulus().data(), keyData.modulus().size(), > > In fact, I don't understand why (uint8_t*) is even needed initially. Because data() returns a signed const char *, and const uint8_t* is the same as const unsigned char*. In other words, it's a signed to unsigned type conversion. Comment on attachment 318933 [details] Patch Clearing flags on attachment: 318933 Committed r221152: <http://trac.webkit.org/changeset/221152> All reviewed patches have been landed. Closing bug. Reopening because: - Partial rollout in <https://trac.webkit.org/changeset/221159> to fix builds. - Found a couple more items that need to be fixed (code movement from WebKit to WebCore). Created attachment 319117 [details]
Part 2 (v1)
Comment on attachment 319117 [details] Part 2 (v1) View in context: https://bugs.webkit.org/attachment.cgi?id=319117&action=review > Source/WebCore/Modules/cache/WorkerCacheStorageConnection.cpp:130 > + m_proxy.postTaskToLoader([protectedThis = makeRef(*this), requestIdentifier, cacheIdentifier](ScriptExecutionContext&) mutable { It is true that capturing this and protectedThis is redundant. Purpose was readability (which is debatable). There are probably other places in the code base that are doing the same thing. Is it the idea to also change these other places? Is the issue that protectedThis being unused is creating warnings? That may be a bummer since there is a pattern to make use of this destroy-lambda-parameters after executing the lambda. This can be used to destroy objects in a specific thread, or the destructor of the object might do a specific job, or we want to keep an object alive without needing to access it directly. Or is clang smart enough to only check for parameters that are not owned by the lambda? Comment on attachment 319117 [details] Part 2 (v1) Attachment 319117 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4383643 New failing tests: imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-delete.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-add.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-storage.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-storage-match.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-keys.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-matchAll.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html Created attachment 319125 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 319117 [details] Part 2 (v1) Attachment 319117 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4383646 New failing tests: imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-delete.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-add.https.html http/wpt/cache-storage/cache-put-keys.https.any.worker.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-storage.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-storage-match.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-keys.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-matchAll.https.html imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html Created attachment 319126 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Let's just close this one, and open a new bug for the remaining patch. It will be less confusing that way. |