[PATCH] D77627: [WebAssembly][MC] Fix leak of std::string members in MCSymbolWasm
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 7 11:57:56 PDT 2020
sbc100 marked an inline comment as done.
sbc100 added inline comments.
================
Comment at: llvm/include/llvm/MC/MCSymbolWasm.h:26-28
+ std::string *ImportModule = nullptr;
+ std::string *ImportName = nullptr;
+ std::string *ExportName = nullptr;
----------------
dblaikie wrote:
> Do these need to be std::strings, or could they just be StringRefs?
>
> It looks like they're only set/get in this class, never modified by it - StringRefs might be more suitable? (does the empty string need to be a separate state from "none"/null - I'd probably still go with Optional<StringRef> then - though opinions certainly might differ there - in that case (where opinions differ & you prefer std::string* over Optional<StringRef> here) I'd at least make them `const std::string*`))
Hm, you are correct. I'm not sure why I didn't go with `Optional<StringRef>`. I'll update.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77627/new/
https://reviews.llvm.org/D77627
More information about the llvm-commits
mailing list