Summary: | Avoid defaulting to capture-by-value for C++11 lambdas in WebCore | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||
Component: | WebCore Misc. | Assignee: | Zan Dobersek <zan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Zan Dobersek
2014-10-09 13:12:49 PDT
Created attachment 239563 [details]
Patch
Attachment 239563 [details] did not pass style-queue:
ERROR: Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:227: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:333: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:375: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h:78: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 5 in 16 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 239563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239563&action=review Outlined a few problems that could have been avoided, most likely of my doing. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:422 > - m_loaderProxy.postTaskToLoader([=](ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, urlCopy, protocolCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->connect(url, protocol); > + peer->connect(urlCopy, protocolCopy); Case in point. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:439 > - m_loaderProxy.postTaskToLoader([=](ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, messageCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->send(message); > + peer->send(messageCopy); Ditto. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:558 > - m_loaderProxy.postTaskToLoader([=] (ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, reasonCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->fail(reason); > + peer->fail(reasonCopy); Ditto. > Source/WebCore/dom/StringCallback.cpp:43 > RefPtr<StringCallback> protector(this); > - context->postTask([=] (ScriptExecutionContext&) { > - this->handleEvent(data); > + context->postTask([protector, data] (ScriptExecutionContext&) { > + protector->handleEvent(data); Ditto. Comment on attachment 239563 [details]
Patch
I like this better. I remember being a little puzzled by the [=] syntax earlier, since I have always used the exhaustive list of what to capture.
Comment on attachment 239563 [details] Patch Clearing flags on attachment: 239563 Committed r174581: <http://trac.webkit.org/changeset/174581> All reviewed patches have been landed. Closing bug. |