Bug 232656 - [WebIDL] A default value = {} in a dictionary is not supported
Summary: [WebIDL] A default value = {} in a dictionary is not supported
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dan Glastonbury
URL:
Keywords: InRadar
Depends on:
Blocks: 232558
  Show dependency treegraph
 
Reported: 2021-11-02 22:37 PDT by Myles C. Maxfield
Modified: 2022-09-03 16:46 PDT (History)
12 users (show)

See Also:


Attachments
Patch (1.53 KB, patch)
2022-03-04 21:40 PST, Dan Glastonbury
no flags Details | Formatted Diff | Diff
Patch (9.61 KB, patch)
2022-03-08 18:18 PST, Dan Glastonbury
no flags Details | Formatted Diff | Diff
Patch (9.60 KB, patch)
2022-03-08 18:29 PST, Dan Glastonbury
sam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.