<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Parse the entire WebAssembly modules"
href="https://bugs.webkit.org/show_bug.cgi?id=147393#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Parse the entire WebAssembly modules"
href="https://bugs.webkit.org/show_bug.cgi?id=147393">bug 147393</a>
from <span class="vcard"><a class="email" href="mailto:sukolsak@gmail.com" title="Sukolsak Sakshuwong <sukolsak@gmail.com>"> <span class="fn">Sukolsak Sakshuwong</span></a>
</span></b>
<pre>Thanks.
(In reply to <a href="show_bug.cgi?id=147393#c3">comment #3</a>)
<span class="quote">> 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?</span >
Fixed.
<span class="quote">> > 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.</span >
Fixed.
(In reply to <a href="show_bug.cgi?id=147393#c5">comment #5</a>)
<span class="quote">> > (In reply to <a href="show_bug.cgi?id=147393#c3">comment #3</a>)
> > > > 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>.</span >
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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>