[PATCH] D66825: [10/10] [LLD] [COFF] Implement MinGW default manifest handling

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked an inline comment as done.
mstorsjo added inline comments.


================
Comment at: llvm/lib/Object/WindowsResource.cpp:316
+         Entry.getNameID() == /* CREATEPROCESS_MANIFEST_RESOURCE_ID */ 1 &&
+         Entry.getLanguage() == 0;
+}
----------------
thakis wrote:
> mstorsjo wrote:
> > thakis wrote:
> > > Should these check for mingw? A normal user-provided resource could probably conceivably have language id 0 as well?
> > Ideally yes, but we don't have the flag available here. We'd need to add it as parameter to the parse functions, passed down recursively. As the harm from it here is pretty small (only one missed warning/error in an obscure corner case) I thought it wasn't worth the churn. But I can add a parameter if you want to.
> Can't you make it a member variable on WindowsResourceParser?
Yes, that works and is simpler. I think I'll push it with a flag named mingw; otherwise the correct name would be "ignoreManifestLangZeroDuplicates", which is more than a mouthful...


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

https://reviews.llvm.org/D66825