[Webkit-unassigned] [Bug 219192] [WASM-References] Add support for active mods in element section

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=219192

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ysuzuki at apple.com
 Attachment #415097|review?                     |review+, commit-queue-
              Flags|                            |

--- Comment #11 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 415097
  --> https://bugs.webkit.org/attachment.cgi?id=415097
Patch

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

r=me with suggestions.

> Source/JavaScriptCore/wasm/WasmFormat.h:233
> +    constexpr static uint32_t NullFuncIndex = UINT32_MAX;

Let's define it as `nullFuncIndex`, which is WebKit's naming convention for constants.

> Source/JavaScriptCore/wasm/WasmFormat.h:235
> +    enum class Kind {

Let's make it `enum class Kind : uint8_t`.

> Source/JavaScriptCore/wasm/WasmFormat.h:248
> +    bool active() const { return kind == Kind::Active; }

isActive() would be better.

> Source/JavaScriptCore/wasm/WasmFormat.h:253
> +    TableElementType elemType;

Let's name it elementType.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:417
> +            // https://bugs.webkit.org/show_bug.cgi?id=219297

Add FIXME with comment about what is missing.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:428
> +            uint8_t elemKind;

=> elementKind.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:450
> +            // https://bugs.webkit.org/show_bug.cgi?id=219385

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:480
> +            // https://bugs.webkit.org/show_bug.cgi?id=219297

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:516
> +            // https://bugs.webkit.org/show_bug.cgi?id=219385

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.h:75
> +    PartialResult WARN_UNUSED_RETURN parseElemKind(uint8_t& elemKind);
> +    PartialResult WARN_UNUSED_RETURN parseIndexCountForElemSection(uint32_t&, const unsigned);
> +    PartialResult WARN_UNUSED_RETURN parseFuncIdxFromRefExpForElemSection(uint32_t&, const unsigned, const unsigned);
> +    PartialResult WARN_UNUSED_RETURN parseFuncIdxForElemSection(uint32_t&, const unsigned, const unsigned);

Let's rename Elem to Element. WebKit prefers non-abbreviated names :)
And since we have parseElement, using "Element" is good for consistency.

> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:558
> +            if (element.active()) {

if (!element.isActive())
    continue;

can be cleaner.

> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:619
>              // for the import.

Let's move `uint32_t functionIndex = element.functionIndices[i];` to the prologue of this loop body and use `functionIndex` instead of `element.functionIndices[i]`.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201202/ad196e47/attachment-0001.htm>