[PATCH] D37240: Fix crbug 759265 by suppressing llvm mt warnings.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Generally looking good. A few minor comments.



================
Comment at: lld/COFF/DriverUtils.cpp:362
 
-static Expected<std::unique_ptr<MemoryBuffer>>
-createManifestXmlWithInternalMt(std::string &DefaultXml) {
+static std::string createManifestXmlWithInternalMt(std::string &DefaultXml) {
   std::unique_ptr<MemoryBuffer> DefaultXmlCopy =
----------------
Can you pass `StringRef` instead of `std::string`? If not, can you make it `const`?


================
Comment at: lld/COFF/DriverUtils.cpp:377
 
-  return Merger.getMergedManifest();
+  return Merger.getMergedManifest().get()->getBuffer();
 }
----------------
This is probably a sign that `getMergedManifest` should return a std::string instead of `std::unique_ptr<MemoryBuffer>`, but this doesn't have to be addressed in this patch.


================
Comment at: lld/COFF/DriverUtils.cpp:380
 
-static std::unique_ptr<MemoryBuffer>
-createManifestXmlWithExternalMt(std::string &DefaultXml) {
-  const Triple HostTriple(Triple::normalize(LLVM_HOST_TRIPLE));
-  if (!HostTriple.isOSWindows())
-    fatal("manifest ignored because no external manifest tool available");
+static std::string createManifestXmlWithExternalMt(std::string &DefaultXml) {
   // Create the default manifest file as a temporary file.
----------------
Ditto


================
Comment at: lld/COFF/DriverUtils.cpp:406-407
+  return check(MemoryBuffer::getFile(User.Path), "could not open " + User.Path)
+      .get()
+      ->getBuffer();
 }
----------------
Is this what clang-format formatted?


https://reviews.llvm.org/D37240