Bug 235023 – Fix C++20 build warnings with GCC

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Lauro Moura]

These warnings appeared after r287698 (the first stint of bug233963 in the tree before being reverted for small adjustments).

They're mostly about:

* `this` not being captured by default in [=] lambdas. (Largely the most common issue)
* Operations between enums of different types are deprecated.

[Lauro Moura]

Created attachment 448722 [details]
Patch

[Lauro Moura]

Oops. Thought that by adding the bug dependency it would pull the patch from the dependency.

[Alex Christensen]

Comment on attachment 448722 [details]
Patch

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

> Source/JavaScriptCore/assembler/X86Assembler.h:338
> +        return (TwoByteOpcodeID)(static_cast<int>(OP2_CMOVCC) + static_cast<int>(cond));

I think instead of adding two static_casts, we could just move the cast to static_cast<TwoByteOpcodeID>(cond)

> Source/WebCore/dom/ViewportArguments.h:77
> +    static constexpr int ValueAuto = -1;

Can we make this an enum class instead of just making them all an int with no type?

> Source/WebCore/dom/WheelEvent.h:35
> +    static const char TickMultiplier = 120;

constexpr
Also char isn't great for this.  Could it be a size_t?

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:641
> +    static constexpr int MaxDimenstionForDirectCompositing = 2000;

float?

[Lauro Moura]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 448722 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448722&action=review
> 
> > Source/JavaScriptCore/assembler/X86Assembler.h:338
> > +        return (TwoByteOpcodeID)(static_cast<int>(OP2_CMOVCC) + static_cast<int>(cond));
> 
> I think instead of adding two static_casts, we could just move the cast to
> static_cast<TwoByteOpcodeID>(cond)

Right, will do.

> 
> > Source/WebCore/dom/ViewportArguments.h:77
> > +    static constexpr int ValueAuto = -1;
> 
> Can we make this an enum class instead of just making them all an int with
> no type?

Wouldn't making it an enum class require static_casts to be used when doing operations with the raw types, even when float is their underlying type? (See https://godbolt.org/z/sqj88fM63 )

Btw, still regarding this enum, looks like `ValueLandscape` and `ValuePortrait` can be removed, as they aren't used anywhere since r256477 / 220631@main.

> 
> > Source/WebCore/dom/WheelEvent.h:35
> > +    static const char TickMultiplier = 120;
> 
> constexpr
> Also char isn't great for this.  Could it be a size_t?

It's used with an IntPoint in WheelEvent.cpp, so I've changed it to int to avoid "narrowing conversions" warnings.

> 
> > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:641
> > +    static constexpr int MaxDimenstionForDirectCompositing = 2000;
> 
> float?

Well spotted.

[Lauro Moura]

Created attachment 449243 [details]
Updated patch after rebase

[EWS]

Committed r288085 (246099@main): <https://commits.webkit.org/246099@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/87664817>