This is an archive of the discontinued LLVM Phabricator instance.

[LLD][COFF] Add support for /noimplib
ClosedPublic

Authored by thieta on Apr 12 2022, 5:03 AM.

Details

Summary

Mostly for compatibility reasons with link.exe this flag
makes sure we don't write a implib - not even when /implib
is also passed, that's how link.exe works.

Diff Detail

Event Timeline

thieta created this revision.Apr 12 2022, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2022, 5:03 AM
thieta requested review of this revision.Apr 12 2022, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2022, 5:03 AM
mstorsjo added inline comments.Apr 12 2022, 5:14 AM
lld/COFF/Options.td
60

Import, not implementation

60

Also “an”, not “a”

thieta updated this revision to Diff 422189.Apr 12 2022, 5:20 AM

Fix help text

thieta marked 2 inline comments as done.Apr 12 2022, 5:20 AM
thieta added inline comments.
lld/COFF/Options.td
60

Thanks! Fixed.

thieta updated this revision to Diff 422221.Apr 12 2022, 7:08 AM
thieta marked an inline comment as done.

Remove unintentional change

mstorsjo accepted this revision.Apr 12 2022, 2:58 PM

LGTM, this looks straightforward to me.

This revision is now accepted and ready to land.Apr 12 2022, 2:58 PM
This revision was automatically updated to reflect the committed changes.
nikic added a subscriber: nikic.Apr 13 2022, 1:47 AM
nikic added inline comments.
lld/COFF/Driver.cpp
1677

nit: Variable arg is unused and causes a warning. Should this be using arg->getValue()?

mstorsjo added inline comments.Apr 13 2022, 2:08 AM
lld/COFF/Driver.cpp
1677

I guess this should be args.hasArg(OPT_noimplib) instead.

@thieta, what happens if the user specifies -noimplib -implib:otherfile.lib? Does the last argument win (i.e. we should use args.getLastArg(OPT_implib, OPT_noimplib) and see which one was set last, or is it the case that -noimplib always trumps -implib, regardless of their respective ordering?

I'll take another pass after lunch - will verify what link does as well. If there is no change in functionality- can that be submitted as a NFC or should I open another review.

I'll take another pass after lunch - will verify what link does as well. If there is no change in functionality- can that be submitted as a NFC or should I open another review.

Yeah if it's just a NFC change getting rid of the warning, I'd say go ahead and push without review here.

I'll take another pass after lunch - will verify what link does as well. If there is no change in functionality- can that be submitted as a NFC or should I open another review.

Yeah if it's just a NFC change getting rid of the warning, I'd say go ahead and push without review here.

I checked and they behave the same. I pushed a NFC for the warning here https://github.com/llvm/llvm-project/commit/837d16fb4c1cb00c5d0c72a0dff543f0bd5ff770