Bug 191583 – Various compiler warnings/errors fixes.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Charlie Turner]

Various compiler warnings/errors fixes.

[Charlie Turner]

Created attachment 354661 [details]
Patch

[Charlie Turner]

The build errors came from https://bugs.webkit.org/show_bug.cgi?id=90342, in release+logging configurations, the others were some warnings I noticed in a wayland upgrade.

[Charlie Turner]

I had a dirty jhbuild for the wayland related changes, rebuilding that here to check the EWS failure.

[Frédéric Wang (:fredw)]

Comment on attachment 354661 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:654
>  #endif

I think the !LOG_DISABLED is for compositingLogEnabled() and ENABLE(TREE_DEBUGGING) for showPaintOrderTree.
Does ENABLE(TREE_DEBUGGING) imply !LOG_DISABLED?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:730
>          showPaintOrderTree(m_renderView.layer());

Ditto.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:765
> +#if ENABLE(TREE_DEBUGGING)

Ditto.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:788
> +#endif

Shouldn't it be

#if !LOG_DISABLED
  if (compositingLogEnabled())
    LOG(Compositing, ...)
#endif

?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1003
> +#endif

Ditto.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1019
> +#endif

Ditto.

> Tools/wpe/backends/WindowViewBackend.cpp:384
> +    [](void*, struct wl_touch*, int32_t, wl_fixed_t) { }

The WPE bot does not seem to like this... Apparently there was a trailing comma in the previous code.

[Charlie Turner]

Comment on attachment 354661 [details]
Patch

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

>> Source/WebCore/rendering/RenderLayerCompositor.cpp:654
>>  #endif
> 
> I think the !LOG_DISABLED is for compositingLogEnabled() and ENABLE(TREE_DEBUGGING) for showPaintOrderTree.
> Does ENABLE(TREE_DEBUGGING) imply !LOG_DISABLED?

TREE_DEBUGGING -> !NDEBUG -> !ASSERTIONS_DISABLED_DEFAULT -> !LOG_DISABLED from my analysis.

>> Source/WebCore/rendering/RenderLayerCompositor.cpp:788
>> +#endif
> 
> Shouldn't it be
> 
> #if !LOG_DISABLED
>   if (compositingLogEnabled())
>     LOG(Compositing, ...)
> #endif
> 
> ?

That seems more robust. I was relying on compositingState.depth only being defined in TREE_DEBUGGING mode, so this checked seemed reasonable. Will update to your suggestion.

>> Tools/wpe/backends/WindowViewBackend.cpp:384
>> +    [](void*, struct wl_touch*, int32_t, wl_fixed_t) { }
> 
> The WPE bot does not seem to like this... Apparently there was a trailing comma in the previous code.

Strange that my compiler didn't mind it :/ seems other inline structures in this file don't need the trailing comma, will double check here.

[Charlie Turner]

Comment on attachment 354661 [details]
Patch

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

>>> Source/WebCore/rendering/RenderLayerCompositor.cpp:788
>>> +#endif
>> 
>> Shouldn't it be
>> 
>> #if !LOG_DISABLED
>>   if (compositingLogEnabled())
>>     LOG(Compositing, ...)
>> #endif
>> 
>> ?
> 
> That seems more robust. I was relying on compositingState.depth only being defined in TREE_DEBUGGING mode, so this checked seemed reasonable. Will update to your suggestion.

Actually, that doesn't work properly. The issue is RenderLayerCompositor::CompositingState.depth is not defined when not building in TREE_DEBUGGING, so accessing it without this guard is incorrect.

[Charlie Turner]

Created attachment 354662 [details]
Patch

Add trailing comma to see if it helps the EWS bots in the wayland part of the patch. Can't reproduce that build failure locally with an updated jhbuild.

[Charlie Turner]

Created attachment 354663 [details]
Patch

There must be something out of sync with the trunk jhbuild and the wpe bot's jhbuild, rolling out the local warning fix.

[Charlie Turner]

Created attachment 354664 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 354664 [details]
Patch for landing

Clearing flags on attachment: 354664

Committed r238130: <https://trac.webkit.org/changeset/238130>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46026592>