Bug 232656

Summary: [WebIDL] A default value = {} in a dictionary is not supported
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: BindingsAssignee: Dan Glastonbury <djg>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, ap, cdumez, djg, esprehn+autocc, ews-watchlist, heycam, kondapallykalyan, rniwa, sam, vitor.roriz, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 232558    
Attachments:
Description Flags
Patch
none
Patch
none
Patch sam: review+, ews-feeder: commit-queue-

Description Myles C. Maxfield 2021-11-02 22:37:27 PDT
dictionary GPUDepthStencilState {
    GPUStencilFaceState stencilFront = {};
}

The "= {}" is not supported.
Comment 1 Myles C. Maxfield 2021-11-03 01:10:21 PDT
Also the "= {}" in

interface GPUCommandEncoder {
    GPUComputePassEncoder beginComputePass(optional GPUComputePassDescriptor descriptor = {});
Comment 2 Radar WebKit Bug Importer 2021-11-09 21:38:20 PST
<rdar://problem/85238647>
Comment 3 Sam Weinig 2022-03-03 10:28:13 PST
In most cases, we do the right thing if just remove the "= { }" because most interfaces treat `nullopt` and default constructed inner type the same.

That said, this would certainly be nice to fix.
Comment 4 Dan Glastonbury 2022-03-03 22:07:31 PST
(In reply to Sam Weinig from comment #3)
> In most cases, we do the right thing if just remove the "= { }" because most
> interfaces treat `nullopt` and default constructed inner type the same.
> 
> That said, this would certainly be nice to fix.

I implemented this by adding the parsing for = {} and returning the "default" value as "{}".  Do I need to update the generator if "we do the right thing..."? (I saw we handle $default == "[]")
Comment 5 Sam Weinig 2022-03-04 16:35:55 PST
If you just want to add the parsing now that is probably fine. I can add the code generator support at some point.
Comment 6 Dan Glastonbury 2022-03-04 21:32:13 PST
Ok, (In reply to Sam Weinig from comment #5)
> If you just want to add the parsing now that is probably fine. I can add the
> code generator support at some point.

Thanks. I've added the parsing (which was pretty simple) and have the parse return undef so the = {} is effectively accepted and thrown away.
Comment 7 Dan Glastonbury 2022-03-04 21:40:44 PST
Created attachment 453901 [details]
Patch
Comment 8 Sam Weinig 2022-03-08 17:05:37 PST
Comment on attachment 453901 [details]
Patch

Can you add a test?
Comment 9 Dan Glastonbury 2022-03-08 18:18:40 PST
Created attachment 454181 [details]
Patch
Comment 10 Dan Glastonbury 2022-03-08 18:24:23 PST
Now with added testing.
Comment 11 Dan Glastonbury 2022-03-08 18:29:35 PST
Created attachment 454182 [details]
Patch
Comment 12 Ahmad Saleem 2022-09-02 15:49:55 PDT
This r+ but I checked on Webkit Github that it has not landed and noticed that it failed some test cases.

Is this required? Thanks!
Comment 13 Dan Glastonbury 2022-09-02 19:42:27 PDT
Pull request: https://github.com/WebKit/WebKit/pull/3982
Comment 14 EWS 2022-09-03 16:46:12 PDT
Committed 254138@main (551709dd217f): <https://commits.webkit.org/254138@main>

Reviewed commits have been landed. Closing PR #3982 and removing active labels.