Bug 226433 - Remove WTF::Optional synonym for std::optional, using that class template directly instead
Summary: Remove WTF::Optional synonym for std::optional, using that class template dir...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P3 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 226437
  Show dependency tree / graph
 
Reported: 2021-05-29 22:36 PDT by Darin Adler
Modified: 2021-06-01 20:56 PDT (History)
71 users (show)

See Also:

Attachments
Patch (4.80 MB, patch)
2021-05-29 23:02 PDT, Darin Adler 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (4.81 MB, patch)
2021-05-29 23:31 PDT, Darin Adler 		cdumez: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (4.81 MB, patch)
2021-05-30 00:02 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2021-05-29 22:36:56 PDT 
Remove WTF::Optional synonym for std::optional, using that class template directly instead
Comment 1 Darin Adler 2021-05-29 23:02:49 PDT Comment hidden (obsolete)
Comment 2 EWS Watchlist 2021-05-29 23:05:10 PDT 
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Comment 3 Darin Adler 2021-05-29 23:31:09 PDT 
Created attachment 430131 [details]
Patch
Comment 4 Chris Dumez 2021-05-29 23:37:49 PDT 
Comment on attachment 430131 [details]
Patch

R=me once the bots are happy.
Comment 5 Darin Adler 2021-05-30 00:02:43 PDT 
Created attachment 430132 [details]
Patch
Comment 6 EWS 2021-05-30 09:12:09 PDT 
Committed r278253 (238290@main): <https://commits.webkit.org/238290@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430132 [details].
Comment 7 Radar WebKit Bug Importer 2021-05-30 09:13:20 PDT 
<rdar://problem/78664888>
Comment 8 Chris Dumez 2021-05-30 12:02:53 PDT 
Seems to have broken the Windows build:
https://build.webkit.org/#/builders/67/builds/3216/steps/8/logs/errors

The issue is not super obvious to me.
Comment 9 Chris Dumez 2021-05-30 12:10:15 PDT 
(In reply to Chris Dumez from comment #8)
> Seems to have broken the Windows build:
> https://build.webkit.org/#/builders/67/builds/3216/steps/8/logs/errors
> 
> The issue is not super obvious to me.

The include is there so I am wondering if there is a `#define optional` somewhere that is polluting us?
Comment 10 Chris Dumez 2021-05-30 12:37:42 PDT 
(In reply to Chris Dumez from comment #9)
> (In reply to Chris Dumez from comment #8)
> > Seems to have broken the Windows build:
> > https://build.webkit.org/#/builders/67/builds/3216/steps/8/logs/errors
> > 
> > The issue is not super obvious to me.
> 
> The include is there so I am wondering if there is a `#define optional`
> somewhere that is polluting us?

Or maybe that bot just needs a clean build. Some of the build output still refers to "Optional":
'Inspector::Protocol::Helpers::Optional': definition of dllimport data not allowed
Comment 11 Darin Adler 2021-05-30 13:51:38 PDT 
Maybe Windows uses a different "erb" file? The EWS Windows build didn’t fail, though!
Comment 12 Chris Dumez 2021-05-30 13:57:26 PDT 
(In reply to Darin Adler from comment #11)
> Maybe Windows uses a different "erb" file? The EWS Windows build didn’t
> fail, though!

And the boy recovered on its own on the next build. It was an incremental build issue I believe.
Comment 13 Chris Dumez 2021-06-01 12:04:43 PDT 
Follow-up build fix in <https://commits.webkit.org/r278319>.
Comment 14 Chris Dumez 2021-06-01 12:09:48 PDT 
(In reply to Chris Dumez from comment #13)
> Follow-up build fix in <https://commits.webkit.org/r278319>.

And <https://commits.webkit.org/r278320>
Comment 15 Michael Catanzaro 2021-06-01 18:09:01 PDT 
Very nice!
Comment 16 Simon Fraser (smfr) 2021-06-01 20:43:49 PDT 
Will the style checker tell me if I use the old thing? A webkit-dev email is probably justified too.
Comment 17 Darin Adler 2021-06-01 20:45:37 PDT 
We can add it to the style checker, but also it won’t compile.
Comment 18 Simon Fraser (smfr) 2021-06-01 20:47:58 PDT 
Not compiling seems sufficient. But a webkit-dev heads-up would be welcome.
Comment 19 Darin Adler 2021-06-01 20:56:17 PDT 
(In reply to Simon Fraser (smfr) from comment #18)
> Not compiling seems sufficient. But a webkit-dev heads-up would be welcome.

https://lists.webkit.org/pipermail/webkit-dev/2021-June/031875.html