[webkit-reviews] review granted: [Bug 199634] [WHLSL] Make enums work : [Attachment 374288] patch

[bugzilla-daemon at webkit.org]

Robin Morisset <rmorisset at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 199634: [WHLSL] Make enums work
https://bugs.webkit.org/show_bug.cgi?id=199634

Attachment 374288: patch

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




--- Comment #5 from Robin Morisset <rmorisset at apple.com> ---
Comment on attachment 374288
  --> https://bugs.webkit.org/attachment.cgi?id=374288
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! 👍