[Webkit-unassigned] [Bug 147393] Parse the entire WebAssembly modules

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
Thanks.

(In reply to comment #3)
> In general, we keep one file per class in WebKit. Since these structs are
> separate from JSWASMModule, they should belong somewhere else.
> 
> It would be tedious to use one file per class for each of these structs, so
> let's use a shared file for all of the structs that define the format of a
> WASM tree. How about WASMFormat.h, as you suggested previously?

Fixed.

> > Source/JavaScriptCore/wasm/JSWASMModule.h:113
> > +    friend class WASMModuleParser;
> 
> "friend" tends toward spaghetti code. We use classes to establish a
> separation of concerns, so that we can think about one class, and change it,
> without worrying about all the other code that uses it. "friend" erases that
> separation and makes our code more brittle.
> 
> Instead of friend, in this case I recommend providing accessors methods for
> these underlying vectors.

Fixed.

(In reply to comment #5)
> > (In reply to comment #3)
> > > > Source/JavaScriptCore/wasm/WASMModuleParser.h:60
> > > > +    JSWASMModule* m_module;
> > > 
> > > You need to do something to prevent m_module from being garbage collected.
> > 
> > Since WASMModuleParser is allocated on the stack, doesn't that prevent
> > m_module from being GC'd? Do I need to do something extra to protect it?
> 
> In practice, you can probably produce GC safety by allocating
> WASMModuleParser on the stack. But that's a dangerous thing to do because
> there's no way to prevent a future coder from moving WASMModuleParser to the
> heap, and in general, C++ is intentionally agnostic about whether an object
> was instantiated on the stack or the heap.
> 
> I think a better solution is to use Strong<T>.

Fixed. I guess one way to do it without using Strong<T> is probably

JSWASMModule* WASMModuleParser::parse(...)
{
    JSWASMModule* module = JSWASMModule::create(...);
    m_module = module;
    ...
    return module;
}

but it seems very hacky.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150731/0513c6c4/attachment.html>