Bug 182797

Summary: Make maximumParallelReadCount static to fix lambda capture error in WebKit::NetworkCache::Storage::traverse()
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit2Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, cdumez, cgarcia, ews-watchlist, mcatanzaro, ross.kirsling, thorton, webkit-bug-importer, Yousuke.Kimoto
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=182679
Attachments:
Description Flags
Patch
none
Patch thorton: review+

Description John Wilander 2018-02-14 10:51:59 PST
https://trac.webkit.org/changeset/228455 added a lambda capture of const unsigned maximumParallelReadCount. Such a capture is not needed and causes a build error when you run with -Wunused-lambda-capture:

WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:911:79: error: lambda capture 'maximumParallelReadCount' is not required to be captured for this use [-Werror,-Wunused-lambda-capture]
            traverseOperation.activeCondition.wait(lock, [&traverseOperation, maximumParallelReadCount] {
                                                                              ^
1 error generated.
Comment 1 Radar WebKit Bug Importer 2018-02-14 10:54:58 PST
<rdar://problem/37540594>
Comment 2 John Wilander 2018-02-14 11:11:08 PST
Created attachment 333820 [details]
Patch
Comment 3 Andy Estes 2018-02-14 11:22:11 PST
Comment on attachment 333820 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333820&action=review

> Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp:911
> -            traverseOperation.activeCondition.wait(lock, [&traverseOperation, maximumParallelReadCount] {
> +            traverseOperation.activeCondition.wait(lock, [&traverseOperation] {

Seems like a clang bug if it's warning about maximumParallelReadCount being unused. It's clearly used!
Comment 4 John Wilander 2018-02-14 11:24:33 PST
Yeah. I wonder if clang optimizes it and moved it into the lambda?
Comment 5 Andy Estes 2018-02-14 11:25:30 PST
This patch might be ok if GCC allows maximumParallelReadCount to be used without being captured. We should see what EWS says.

Otherwise, we might need to either move the definition of maximumParallelReadCount into the lambda, or make it static.
Comment 6 John Wilander 2018-02-14 11:30:07 PST
Created attachment 333823 [details]
Patch
Comment 7 John Wilander 2018-02-14 11:37:14 PST
WPE tree seems red.
Comment 8 John Wilander 2018-02-14 11:43:45 PST
Comment on attachment 333823 [details]
Patch

Thanks, Tim and Andy!
Comment 9 John Wilander 2018-02-14 11:47:59 PST
Committed r228478: <https://trac.webkit.org/changeset/228478>