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

[Nico Weber via Phabricator via llvm-commits]

thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

lgtm, with a suggestion for an (I think?) missing test:



================
Comment at: lld/COFF/Driver.cpp:1012
+          toString(resourceObjFiles.front()));
+    return;
   }
----------------
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.


================
Comment at: lld/test/COFF/combined-resources.test:32
+# RUN: cmp %t-orig.txt %t-resobj.txt
+
 CHECK:      ResourceTableRVA: 0x2000
----------------
You could add a test where there's a dupe resource spread over two .res / res-.obj files.


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

https://reviews.llvm.org/D66824