Bug 222351 - [WASM-Function-References] Extend wasm type with type index
Summary: [WASM-Function-References] Extend wasm type with type index
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebAssembly (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 247393
  Show dependency tree / graph
 
Reported: 2021-02-24 03:39 PST by Dmitry
Modified: 2022-11-02 17:08 PDT (History)
8 users (show)

See Also:

Attachments
Patch (178.54 KB, patch)
2021-02-24 03:42 PST, Dmitry 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry 2021-02-24 03:39:53 PST 
[WASM-Function-References] Extend wasm type with type index
Comment 1 Dmitry 2021-02-24 03:42:21 PST 
Created attachment 421394 [details]
Patch
Comment 2 Keith Miller 2021-02-26 09:37:58 PST 
Comment on attachment 421394 [details]
Patch

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

r=me.

> Source/JavaScriptCore/wasm/generateWasmOpsHeader.py:219
> +    bool operator==(const Type& other) const

Seems like it'd be reasonable to have an implicit TypeKind constructor so you can just use the enum literals as Types.
Comment 3 Dmitry 2021-02-28 21:13:44 PST 
There are two reason why we may want not to introduce any user-defined constructors:
1) Now Wasm::Type is a POD type and it is a good property to have, any user-defined constructor breaks this.
2) In the near future not all TypeKind values can represent Wasm::Type, for example - TypeKind::Reference can't represent Type without specifying type_idx

wdyt?
Comment 4 Radar WebKit Bug Importer 2021-03-03 03:40:15 PST 
<rdar://problem/74978305>
Comment 5 EWS 2021-03-03 04:20:05 PST 
Committed r273813: <https://commits.webkit.org/r273813>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421394 [details].