Bug 231239 - Prepare to switch from WTF::Variant to std::variant
Summary: Prepare to switch from WTF::Variant to std::variant
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-05 10:18 PDT by Alex Christensen
Modified: 2021-10-11 10:24 PDT (History)
49 users (show)

See Also:


Attachments
Patch (77.47 KB, patch)
2021-10-05 10:26 PDT, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (77.47 KB, patch)
2021-10-05 10:56 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (77.28 KB, patch)
2021-10-05 12:00 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (72.36 KB, patch)
2021-10-05 15:24 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-10-05 10:18:54 PDT
Prepare to switch from WTF::Variant to std::variant
Comment 1 Alex Christensen 2021-10-05 10:26:48 PDT
Created attachment 440232 [details]
Patch
Comment 2 Alex Christensen 2021-10-05 10:56:43 PDT
Created attachment 440236 [details]
Patch
Comment 3 Darin Adler 2021-10-05 11:19:26 PDT
Comment on attachment 440236 [details]
Patch

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

> Source/JavaScriptCore/runtime/CacheUpdate.cpp:41
>  CacheUpdate::CacheUpdate(CacheUpdate&& other)

Just had another idea for this. Can we try this?

    CacheUpdate::CacheUpdate(CacheUpdate&&) = default;
Comment 4 Darin Adler 2021-10-05 11:28:46 PDT
Comment on attachment 440236 [details]
Patch

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

>> Source/JavaScriptCore/runtime/CacheUpdate.cpp:41
>>  CacheUpdate::CacheUpdate(CacheUpdate&& other)
> 
> Just had another idea for this. Can we try this?
> 
>     CacheUpdate::CacheUpdate(CacheUpdate&&) = default;

We could also experiment with removing the explicit move constructor and assignment operator from the class entirely. I am pretty sure the compiler will generate the correct code if we just don’t say anything.
Comment 5 Alex Christensen 2021-10-05 12:00:42 PDT
Created attachment 440243 [details]
Patch
Comment 6 Alex Christensen 2021-10-05 12:13:01 PDT
Removing it causes compiler errors, but =default works fine.
Comment 7 Alex Christensen 2021-10-05 15:24:44 PDT
Created attachment 440279 [details]
Patch
Comment 8 Xabier Rodríguez Calvar 2021-10-06 02:50:01 PDT
There are several appearances in for example CDMProxy.h and related files that are not being changed. In the GTK/WPE ports, these things are built when experimental features are enabled and I don't recall if we have them enabled in EWS so they won't fail. Anyway, I don't know if you're taking a step by step approach but those would need changing as well or things will break on our side.
Comment 9 Philippe Normand 2021-10-06 02:54:54 PDT
GTK/EWS developer builds have experimental features enabled.
Comment 10 EWS 2021-10-11 10:23:01 PDT
Committed r283906 (242780@main): <https://commits.webkit.org/242780@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440279 [details].
Comment 11 Radar WebKit Bug Importer 2021-10-11 10:24:32 PDT
<rdar://problem/84105079>