Summary: | [WPE] Inline wl_array_for_each to workaround C++ compatibility issue | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Charlie Turner <cturner> | ||||||||
Component: | WPE WebKit | Assignee: | Charlie Turner <cturner> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aperez, bugs-noreply, cgarcia, commit-queue, mcatanzaro, olivier.blin, pagebul, zan | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Charlie Turner
2019-02-21 07:10:50 PST
Created attachment 362603 [details]
Patch
There's an upstream bug report related to this: https://gitlab.freedesktop.org/wayland/wayland/issues/34 As discussed with Charlie in a chat, it seems a tad better to have the “for”-loop expanded by hand in this code instead of relying on the wl_arrat_for_each() macro, instead of disabling the block of code completely. Created attachment 362715 [details]
Patch
Comment on attachment 362715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362715&action=review Informal r+ from me, with a (very) minor nit. > Tools/wpe/backends/WindowViewBackend.cpp:451 > + while (pos < end) { Minor nitpick: I would probably write this as a for-loop with empty initializer list: for (; pos < end; pos++) Though this is mainly personal preference, because I find code easier to follow when the “increment” done on each iteration is written along the header of the loop :) > Tools/wpe/backends/WindowViewBackend.cpp:452 > + uint32_t state = *pos; Alternatively, if you prefer to keep the “while”, you can do: uint32_t state = *pos++; Created attachment 362722 [details]
Patch
Thanks Adrian, I will go with your style and use the for-loop
Comment on attachment 362722 [details] Patch Clearing flags on attachment: 362722 Committed r242344: <https://trac.webkit.org/changeset/242344> All reviewed patches have been landed. Closing bug. *** Bug 194290 has been marked as a duplicate of this bug. *** |