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

Thu Aug 29 13:46:28 PDT 2019

thakis added a comment.

In D66824#1651630 <https://reviews.llvm.org/D66824#1651630>, @mstorsjo wrote:

> In D66824#1651587 <https://reviews.llvm.org/D66824#1651587>, @thakis wrote:
>
> > I'd vote for not running complicated code unless we have to, so I'd keep this MinGW only.
>
>
> TBH it's not all that complicated, but in any case, none of that code should run in the cases that are currently supported anyway. It's just choosing between if you want to keep the error, to keep matching link.exe, or be more tolerant and support multiple resource objects even for non-MinGW setups.

To be honest, I don't have a strong opinion. I can see arguments for both behaviors and I'm not sure which of them are better, assuming we have to have this resource merging code in lld anyways. If there's a way to prevent having to have this merging code and making the error behavior work for mingw, I'd prefer that :)

> 
> 
>> Do you really need this feature in MinGW mode? IIRC you said this was for some "default res file" feature in gcc which adds a default res which is only supposed to be used if there's no user res file -- which doesn't require any merging.
> 
> The user resource object file could be other resources (icons and dialogs and whatnot), where the manifest is supposed to be merged with the rest. Only if the user resources happens to have a manifest should the implicit default one be dropped.

Wouldn't users pass .res files to the linker? If not, why not?

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

https://reviews.llvm.org/D66824