Bug 199634 – [WHLSL] Make enums work

• Component: WebGPU
• Status: RESOLVED FIXED

[Saam Barati]

This shouldn't crash:

        enum Weekday {
            Monday,
            Tuesday,
            Wednesday,
            Thursday,
            Pizzaday
        }
        int foo()
        {
            (Weekday.Monday);
            return 42;
        }

[Saam Barati]

doing this

[Saam Barati]

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

[Saam Barati]

Created attachment 374287 [details]
patch

[Saam Barati]

Created attachment 374288 [details]
patch

[Robin Morisset]

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! 👍

[Saam Barati]

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.

[Saam Barati]

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.

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/247666/webkit

[Radar WebKit Bug Importer]

<rdar://problem/53344896>