[PATCH] D38513: [LLD] [COFF] Add support for GNU binutils import libraries

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D38513#1240693, @ruiu wrote:

> Yeah, I agree with you that this patch is lean. It actually looks pretty good.


Thanks! I'll update it according to your other comments, except for the one I'd prefer avoid changing here.



================
Comment at: COFF/Writer.cpp:636
+    if (!isa<DefinedImportThunk>(File->ThunkSym))
+      fatal(toString(*File->ThunkSym) + " was replaced");
+    DefinedImportThunk *Thunk = cast<DefinedImportThunk>(File->ThunkSym);
----------------
ruiu wrote:
> I wonder if you should use `assert`. If this line is unreachable unless there's a bug in lld, you should use `assert`.
This check is just moved, so I'd avoid changing it within this patch.

I think it might theoretically be possible to trigger it with very pathological input, but I don't remember exactly right now...


https://reviews.llvm.org/D38513