Require unique_ptr to Module::addFunctionType() #1672
Conversation
chfast
force-pushed
the
fix-mem-leak
branch
2 times, most recently
from
45e6861 to
08d85bc
Compare
chfast
force-pushed
the
fix-mem-leak
branch
from
08d85bc to
f54556b
Compare
|
Should I still work on this? |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Sorry for the delay here. Yes, I think this is worth doing, i agree it's better to use unique_ptrs consistently.
chfast
force-pushed
the
fix-mem-leak
branch
2 times, most recently
from
9885681 to
1af5859
Compare
|
Have you joined the w3c community group? (required for contributors) Looks like there are some compilation errors on CI here. |
|
@chfast ping |
I have.
Fixed. |
|
Ping. |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Sorry for the delay here! Somehow I missed this among my emails.
I think this is good, aside for my question about the fillFunction change. It looks separate from the main change here anyhow - could we separate it out? Or is there a connection I am missing?
src/ir/function-type-utils.h
Outdated
| inline void fillFunction(Function* func, FunctionType* type) { | ||
| func->params = type->params; | ||
| func->result = type->result; | ||
| inline void fillFunction(Function& func, const FunctionType& type) { |
There was a problem hiding this comment.
Why is the signature changed here? The downside is that we do tend to use pointers in most places, which means a bunch of extra *, which I think is less readable.
There was a problem hiding this comment.
I removed this commit.
This fixes the memory leak in WasmBinaryBuilder::readSignatures() caused probably the exception thrown there before the FunctionType object is safe. This also makes it clear that the Module becomes the owner of the FunctionType objects.
|
Great, thanks! |
3 participants
This fixes the memory leak in WasmBinaryBuilder::readSignatures() caused probably the exception thrown there before the FunctionType object is safe.
This also makes it clear that the Module becomes the owner of the FunctionType objects.