Bug 199634

Summary: [WHLSL] Make enums work
Product: WebKit Reporter: Saam Barati <saam>
Component: WebGPUAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, fpizlo, jonlee, justin_fan, mmaxfield, rmorisset, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 195681, 199853    
Attachments:
Description Flags
WIP
none
patch
none
patch rmorisset: review+

Description Saam Barati 2019-07-09 12:21:24 PDT
This shouldn't crash:

        enum Weekday {
            Monday,
            Tuesday,
            Wednesday,
            Thursday,
            Pizzaday
        }
        int foo()
        {
            (Weekday.Monday);
            return 42;
        }
Comment 1 Saam Barati 2019-07-15 17:14:36 PDT
doing this
Comment 2 Saam Barati 2019-07-15 17:36:23 PDT
Created attachment 374172 [details]
WIP

WIP. It's starting to work. Just need to write some interesting tests around "overflowing" the +1 of enum values past UINT_MAX/INT_MAX
Comment 3 Saam Barati 2019-07-17 04:22:44 PDT
Created attachment 374287 [details]
patch
Comment 4 Saam Barati 2019-07-17 04:29:27 PDT
Created attachment 374288 [details]
patch
Comment 5 Robin Morisset 2019-07-17 11:24:00 PDT
Comment on attachment 374288 [details]
patch

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

r=me with just a few comments.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:852
> +auto Parser::parseEnumerationMember(int64_t defaltValue) -> Expected<AST::EnumerationMember, Error>

defalt -> default

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:875
> +    return AST::EnumerationMember(*identifier, WTFMove(name), defaltValue);

defalt -> default

> LayoutTests/webgpu/whlsl-enums-2.html:64
> +            return a == b;

So == and != are to be automatically generated for enums? I can add it to the spec if it is on purpose.

> LayoutTests/webgpu/whlsl-enums-2.html:144
> +            return _war().value;

Similarly, this ".value" thing is not in the spec currently. I can add it, but why have both this and int(_war()) be supported?

> LayoutTests/webgpu/whlsl-enums.html:13
> +whlslTests.failTrickyDoubleZero = async () => {

The tests in this file are very nice! 👍
Comment 6 Saam Barati 2019-07-20 05:09:22 PDT
Comment on attachment 374288 [details]
patch

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

>> LayoutTests/webgpu/whlsl-enums-2.html:144
>> +            return _war().value;
> 
> Similarly, this ".value" thing is not in the spec currently. I can add it, but why have both this and int(_war()) be supported?

I just copied this from the spec repo. I vote for removing it unless we think it's necessary. I'll omit these tests for now.
Comment 7 Saam Barati 2019-07-20 05:10:29 PDT
Comment on attachment 374288 [details]
patch

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

>> LayoutTests/webgpu/whlsl-enums-2.html:64
>> +            return a == b;
> 
> So == and != are to be automatically generated for enums? I can add it to the spec if it is on purpose.

Yeah I believe they are.
Comment 8 Saam Barati 2019-07-20 05:41:57 PDT
landed in:
https://trac.webkit.org/changeset/247666/webkit
Comment 9 Radar WebKit Bug Importer 2019-07-20 05:42:33 PDT
<rdar://problem/53344896>