[PATCH] D32689: DEF: migrate def parser from LLD to LLVM

[Martell Malone via Phabricator via llvm-commits]

martell marked an inline comment as done.
martell added a comment.

In https://reviews.llvm.org/D32689#743003, @ruiu wrote:

> Please find the cause of the memory issue and fix it. If you just use `new std::string` and leave it alone, you're leaking memory, which is unacceptable for a library (it's bad even for application).


The only reasonable way I can currently get this running without destructor issues is by changing
`COFFModuleDefinition`'s `Exports` from `std::vector<COFFShortExport>` to `std::vector<COFFShortExport*>` and have the consumer of `COFFModuleDefinition` responsible for the vector of pointers.
With this I can then delete the vector after use in `Driver.cpp`, although it is not the most elegant.

Any attempt to change `E.Name` or `E.Name` within `parseExport` to a local `std::string` will be destroyed.
Using `std::move` prompts an error confirming this.

I have been programming a lot in rust lang recently so my brain is clogged up with thoughts of lifetimes and generics so maybe I am not thinking clearly.
Suggestions are very welcome.



================
Comment at: lib/Object/COFFImportFile.cpp:116
+  }
+  return (Twine(S.substr(0, Pos)) + To + S.substr(Pos + From.size())).getSingleStringRef();
+}
----------------
pcc wrote:
> This is returning a newly created string, so I don't think you can return `Expected<StringRef>` here.
I was able to change this to `Expected<std::string>`


Repository:
  rL LLVM

https://reviews.llvm.org/D32689