Bug 224787 – Remove UNUSED warnings based on the configuration.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Basuke Suzuki]

Many warnings are came from the result when RELEASE_LOG is disabled.

[Basuke Suzuki]

Created attachment 426490 [details]
PATCH

[Basuke Suzuki]

Created attachment 426494 [details]
PATCH

[Basuke Suzuki]

Created attachment 426495 [details]
PATCH

[Darin Adler]

Comment on attachment 426495 [details]
PATCH

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

> Source/WebCore/loader/SubresourceLoader.cpp:311
> +            UNUSED_VARIABLE(this);

This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:540
>      unsigned numberOfBatches = std::ceil(domains.size() / static_cast<float>(maxNumberOfDomainsInOneLogStatement));
> +    UNUSED_VARIABLE(numberOfBatches);

Better fix is to wrap this line in:

#if !RELEASE_LOG_DISABLED
#endif

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:359
> +#endif

I think this #endif should be up above, not wrapped around the entire block. We don’t want or need to nest the #if

> Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160
>      RELEASE_LOG(ServiceWorker, "Created service worker %" PRIu64 " in process PID %i", serviceWorkerIdentifier.toUInt64(), getCurrentProcessID());
> +    UNUSED_VARIABLE(serviceWorkerIdentifier);

Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable.

> Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:55
> +static UNUSED_FUNCTION uint64_t pageIDFromWebFrame(const RefPtr<WebFrame>& frame)

Better fix is to put these functions inside #if !RELEASE_LOG_DISABLED.

[Basuke Suzuki]

Comment on attachment 426495 [details]
PATCH

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

>> Source/WebCore/loader/SubresourceLoader.cpp:311
>> +            UNUSED_VARIABLE(this);
> 
> This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either.

But `this` is uses in RELEASE_LOG_IF_ALLOWED macro. It will be warned if RELEASE_LOG_DISABLED.

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:540
>> +    UNUSED_VARIABLE(numberOfBatches);
> 
> Better fix is to wrap this line in:
> 
> #if !RELEASE_LOG_DISABLED
> #endif

Right.

>> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:359
>> +#endif
> 
> I think this #endif should be up above, not wrapped around the entire block. We don’t want or need to nest the #if

Ah, okay. Make sense.

>> Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160
>> +    UNUSED_VARIABLE(serviceWorkerIdentifier);
> 
> Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable.

Right.

>> Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:55
>> +static UNUSED_FUNCTION uint64_t pageIDFromWebFrame(const RefPtr<WebFrame>& frame)
> 
> Better fix is to put these functions inside #if !RELEASE_LOG_DISABLED.

Right.

[Basuke Suzuki]

Comment on attachment 426495 [details]
PATCH

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

>>> Source/WebCore/loader/SubresourceLoader.cpp:311
>>> +            UNUSED_VARIABLE(this);
>> 
>> This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either.
> 
> But `this` is uses in RELEASE_LOG_IF_ALLOWED macro. It will be warned if RELEASE_LOG_DISABLED.

I will add RELEASE_LOG_IF_ALLOWED for RELEASE_LOG_DISABLED case.

>>> Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160
>>> +    UNUSED_VARIABLE(serviceWorkerIdentifier);
>> 
>> Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable.
> 
> Right.

Ah, no. it is passed with WTFMove(data). We shouldn't use that. Wrap with #!RELEASE_LOG_DISABLED.

[Basuke Suzuki]

Created attachment 426504 [details]
PATCH

[Darin Adler]

Comment on attachment 426504 [details]
PATCH

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

> Source/WebKit/WebProcess/WebProcess.cpp:1469
> +        UNUSED_VARIABLE(this);

Don’t need this any more since you put a fix into RELEASE_LOG_IF_ALLOWED

[Basuke Suzuki]

Comment on attachment 426504 [details]
PATCH

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2214
> +        UNUSED_VARIABLE(this);

Can we do something for this?

>> Source/WebKit/WebProcess/WebProcess.cpp:1469
>> +        UNUSED_VARIABLE(this);
> 
> Don’t need this any more since you put a fix into RELEASE_LOG_IF_ALLOWED

That was defined in SubresourceLoader.cpp, so I have to define that here for this file. Okay.

[Basuke Suzuki]

Created attachment 426518 [details]
PATCH

[EWS]

Committed r276306 (236788@main): <https://commits.webkit.org/236788@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426518 [details].

[Ling Ho]

rdar://76900340