[PATCH] D66824: [09/10] [LLD] [COFF] Support merging resource object files

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1012
+          toString(resourceObjFiles.front()));
+    return;
   }
----------------
thakis wrote:
> mstorsjo wrote:
> > @thakis - The question is only about whether to keep this explicit error here, or remove the whole condition.
> Ack. I'd keep it, but I don't feel strongly.
Ok, I can keep it. It allows multiple-resource-objs.test to keep testing that we don't leave the output file behind as well :-)


================
Comment at: lld/test/COFF/combined-resources.test:32
+# RUN: cmp %t-orig.txt %t-resobj.txt
+
 CHECK:      ResourceTableRVA: 0x2000
----------------
thakis wrote:
> You could add a test where there's a dupe resource spread over two .res / res-.obj files.
Oh, good point, I'll add such a test as well, before committing it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66824/new/

https://reviews.llvm.org/D66824