Summary: | Fix C++20 build warnings with GCC | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Lauro Moura <lmoura> | ||||||
Component: | Tools / Tests | Assignee: | Lauro Moura <lmoura> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, benjamin, cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, glenn, hi, joepeck, kangil.han, keith_miller, kondapallykalyan, luiz, mark.lam, msaboff, pangle, pdr, saam, tzagallo, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 233963 | ||||||||
Bug Blocks: | 195548 | ||||||||
Attachments: |
|
Description
Lauro Moura
2022-01-09 21:26:00 PST
Created attachment 448722 [details]
Patch
Oops. Thought that by adding the bug dependency it would pull the patch from the dependency. 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? (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. Created attachment 449243 [details]
Updated patch after rebase
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]. |