Bug 194898 - [WPE] Inline wl_array_for_each to workaround C++ compatibility issue
Summary: [WPE] Inline wl_array_for_each to workaround C++ compatibility issue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords:
: 194290 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-02-21 07:10 PST by Charlie Turner
Modified: 2019-05-02 16:23 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2019-02-21 07:12 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (2.13 KB, patch)
2019-02-22 04:39 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (1.92 KB, patch)
2019-02-22 07:47 PST, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2019-02-21 07:10:50 PST
[WPE] Condition use of wl_array_for_each on GCC
Comment 1 Charlie Turner 2019-02-21 07:12:53 PST
Created attachment 362603 [details]
Patch
Comment 2 Adrian Perez 2019-02-22 03:36:45 PST
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.
Comment 3 Charlie Turner 2019-02-22 04:39:31 PST
Created attachment 362715 [details]
Patch
Comment 4 Adrian Perez 2019-02-22 05:10:21 PST
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++;
Comment 5 Charlie Turner 2019-02-22 07:47:47 PST
Created attachment 362722 [details]
Patch

Thanks Adrian, I will go with your style and use the for-loop
Comment 7 WebKit Commit Bot 2019-03-04 00:32:24 PST
Comment on attachment 362722 [details]
Patch

Clearing flags on attachment: 362722

Committed r242344: <https://trac.webkit.org/changeset/242344>
Comment 8 WebKit Commit Bot 2019-03-04 00:32:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Olivier Blin 2019-03-04 02:42:18 PST
*** Bug 194290 has been marked as a duplicate of this bug. ***