Bug 198706

Summary: [WHLSL] Remove unnecessary ASSERT()s and clean up visitor lambdas
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, fpizlo, jonlee, justin_fan, rmorisset, saam, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 198704    
Attachments:
Description Flags
WIP
none
Patch
none
Patch dino: review+

Description Myles C. Maxfield 2019-06-09 23:38:45 PDT
General cleanup. No behavior change.
Comment 1 Myles C. Maxfield 2019-06-09 23:39:15 PDT
Created attachment 371724 [details]
WIP
Comment 2 Myles C. Maxfield 2019-06-09 23:40:13 PDT
*** Bug 198705 has been marked as a duplicate of this bug. ***
Comment 3 Jon Lee 2019-06-12 08:53:03 PDT
Changing to P1 since it blocks 198704.
Comment 4 Radar WebKit Bug Importer 2019-06-12 08:54:25 PDT
<rdar://problem/51668851>
Comment 5 Myles C. Maxfield 2019-06-12 23:54:38 PDT
Created attachment 372022 [details]
Patch
Comment 6 Myles C. Maxfield 2019-06-13 10:44:49 PDT
Created attachment 372066 [details]
Patch
Comment 7 Saam Barati 2019-06-13 12:55:30 PDT
LGTM too.
Comment 8 Robin Morisset 2019-06-13 14:08:39 PDT
Comment on attachment 372066 [details]
Patch

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

r=me with just a nitpick.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLIntrinsics.cpp:350
> +    auto textureTypeIndex = std::find(m_textureTypeNames, m_textureTypeNames + WTF_ARRAY_LENGTH(m_textureTypeNames), nativeTypeDeclaration.name()) - m_textureTypeNames;

I understand that is more concise, but I am not entirely comfortable with replacing some reasonable code by pointer arithmetic (the - m_textureTypeNames at the end).
It gets me especially wary because of the auto. Is this std::ptrdiff_t ?uintptr_t ?
Comment 9 Myles C. Maxfield 2019-06-13 22:46:04 PDT
Committed r246428: <https://trac.webkit.org/changeset/246428>