[PATCH] D36255: Integrate manifest merging library into LLD.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 16 15:40:01 PDT 2017


ruiu added inline comments.


================
Comment at: lld/COFF/DriverUtils.cpp:347
 
 static std::string createManifestXml() {
+  std::string DefaultXml = createDefaultXml();
----------------
Separate this function into two functions, createManifestXmlLibxml and createManifestXmlMt, and call them sequentially from createManifestXml.


================
Comment at: lld/COFF/DriverUtils.cpp:362-364
+  if (auto E = Merger.merge(*DefaultXmlCopy.get())) {
+    fatal(E, "failed to merge default manifest");
+  }
----------------
Please follow the coding style in this file, so that the new code looks like they existed from beginning. Remove `{}`.


================
Comment at: lld/COFF/DriverUtils.cpp:373
+
+  OutputBuffer = Merger.getMergedManifest();
+
----------------
Define `OutputBuffer` here instead of at the binning of this function, as this is the first line you are using that variable.


https://reviews.llvm.org/D36255





More information about the llvm-commits mailing list