Bug 226433

Summary: Remove WTF::Optional synonym for std::optional, using that class template directly instead
Product: WebKit Reporter: Darin Adler <darin>
Component: WebKit Misc.Assignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, alecflett, allan.jensen, apinheiro, bburg, beidson, benjamin, berto, calvaris, cdumez, cfleizach, cgarcia, changseok, cmarcelo, dbarton, dino, dmazzoni, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, fred.wang, galpeter, glenn, graouts, gustavo, gyuyoung.kim, hi, hta, jamesr, japhet, jbedard, jcraig, jdiggs, jer.noble, jfernandez, jiewen_tan, joepeck, jsbell, kangil.han, keith_miller, kondapallykalyan, luiz, macpherson, mark.lam, mcatanzaro, menard, mifenton, mmaxfield, msaboff, pdr, philipj, pnormand, rego, ryanhaddad, saam, sabouhallawa, samuel_white, sam, schenney, sergio, simon.fraser, svillar, tommyw, tonikitoo, toyoshim, tzagallo, vjaquez, webkit-bug-importer, youennf, yutak
Priority: P3 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 226437    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
cdumez: review+, ews-feeder: commit-queue-
Patch none

Darin Adler
Reported 2021-05-29 22:36:56 PDT
Remove WTF::Optional synonym for std::optional, using that class template directly instead
Attachments
Patch (4.80 MB, patch)
2021-05-29 23:02 PDT, Darin Adler 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (4.81 MB, patch)
2021-05-29 23:31 PDT, Darin Adler 		cdumez: review+
ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (4.81 MB, patch)
2021-05-30 00:02 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2021-05-29 23:02:49 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 2 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`)
Darin Adler
Comment 3 2021-05-29 23:31:09 PDT
Created attachment 430131 [details] Patch
Chris Dumez
Comment 4 2021-05-29 23:37:49 PDT
Comment on attachment 430131 [details] Patch R=me once the bots are happy.
Darin Adler
Comment 5 2021-05-30 00:02:43 PDT
Created attachment 430132 [details] Patch
EWS
Comment 6 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].
Radar WebKit Bug Importer
Comment 7 2021-05-30 09:13:20 PDT
<rdar://problem/78664888>
Chris Dumez
Comment 8 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.
Chris Dumez
Comment 9 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?
Chris Dumez
Comment 10 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
Darin Adler
Comment 11 2021-05-30 13:51:38 PDT
Maybe Windows uses a different "erb" file? The EWS Windows build didn’t fail, though!
Chris Dumez
Comment 12 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.
Chris Dumez
Comment 13 2021-06-01 12:04:43 PDT
Follow-up build fix in <https://commits.webkit.org/r278319>.
Chris Dumez
Comment 14 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>
Michael Catanzaro
Comment 15 2021-06-01 18:09:01 PDT
Very nice!
Simon Fraser (smfr)
Comment 16 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.
Darin Adler
Comment 17 2021-06-01 20:45:37 PDT
We can add it to the style checker, but also it won’t compile.
Simon Fraser (smfr)
Comment 18 2021-06-01 20:47:58 PDT
Not compiling seems sufficient. But a webkit-dev heads-up would be welcome.
Darin Adler
Comment 19 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
Note You need to log in before you can comment on or make changes to this bug.