[PATCH] D43933: [WebAssembly] Pass ownership of body to SyntheticFunction . NFC

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

LGTM

It feels like there's some way to simplify the code in other way though. One idea is to add `OS` member to a synthetic section and add contents to that stream, instead of creating a function body beforehand and pass it to the synthetic function's constructor. Maybe there are other possibilities too. But I don't think that's not very important at the moment.



================
Comment at: wasm/InputChunks.h:156
+  SyntheticFunction(const WasmSignature &S, std::string Body, StringRef Name)
+      : InputFunction(S, nullptr, nullptr), Name(Name), Body(std::move(Body)) {}
 
----------------
sbc100 wrote:
> Seems reasonable.  I'm not sure if the ownership passing and move semantics stuff is overkill here in lld land though.  Seems like make<> and the arenaAllocator might be preferred/simpler.  I'll defer to @ruiu 
Yeah, that's good point. I was thinking about that. In lld, we don't do memory management per object since almost all heap-allocated objects are needed until the last moment when we flush everything to disk. That said, I'm perhaps OK with this. This is a small piece of code, and we can change later when we change the mind.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43933