Bug 182797 - Make maximumParallelReadCount static to fix lambda capture error in WebKit::NetworkCache::Storage::traverse()
Summary: Make maximumParallelReadCount static to fix lambda capture error in WebKit::N...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
Keywords: InRadar
Depends on:
Reported: 2018-02-14 10:51 PST by John Wilander
Modified: 2018-02-14 11:47 PST (History)
9 users (show)

See Also:

Patch (1.56 KB, patch)
2018-02-14 11:11 PST, John Wilander
no flags Details | Formatted Diff | Diff
Patch (1.71 KB, patch)
2018-02-14 11:30 PST, John Wilander
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
Comment 2 John Wilander 2018-02-14 11:11:08 PST
Created attachment 333820 [details]
Comment 3 Andy Estes 2018-02-14 11:22:11 PST
Comment on attachment 333820 [details]

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]
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]

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