This is an archive of the discontinued LLVM Phabricator instance.

[lld-link] Add support for /ignore:4037.
ClosedPublic

Authored by thakis on Mar 8 2018, 9:05 PM.

Details

Reviewers
ruiu
hans
Summary

Fixes PR36657.

Diff Detail

Event Timeline

thakis created this revision.Mar 8 2018, 9:05 PM
hans added a subscriber: hans.Mar 9 2018, 12:01 AM
hans added inline comments.
COFF/Driver.cpp
809

Higher-level question: link.exe prints the warning number here, so the user can figure out what /ignore option to pass. Is there some way we can let an lld-link.exe user figure out how to ignore this warning?

thakis added inline comments.Mar 9 2018, 3:33 AM
COFF/Driver.cpp
809

This is imho an excellent question, and I wondered about this myself. /ignore:4217 (already implemented) has the same problem – so since this is consistent with that warning, I figured I don't need to figure this out right now.

lld-link's warning configurability is generally nonexistent, making this even less obvious. Maybe we should add "[/ignore:N to ignore]" to the end of the warning text for warnings that have a disable flag? That seems like a good approach to me – Rui, what do you think? But that should be a separate patch.

hans accepted this revision.Mar 9 2018, 3:44 AM
hans added inline comments.
COFF/Driver.cpp
809

Yes, absolutely this should be handled separately, just wanted to raise the question.

Your patch looks good to me

This revision is now accepted and ready to land.Mar 9 2018, 3:44 AM
thakis closed this revision.Mar 9 2018, 4:43 AM

r327124, thanks!

aganea added a subscriber: aganea.Mar 10 2018, 7:22 PM

Side note - I raised the /ignore issue here: https://bugs.llvm.org/show_bug.cgi?id=36445

In the future I think this should be fixed once for all by tagging the messages with the corresponding Microsoft LNK number.

In that case, warn("/order...") could become lnkWarn(4217, "/order...") and automatically handle /ignore.

ruiu added a comment.Mar 14 2018, 2:23 PM

In the future I think this should be fixed once for all by tagging the messages with the corresponding Microsoft LNK number.

In that case, warn("/order...") could become lnkWarn(4217, "/order...") and automatically handle /ignore.

Adding a helper function that takes an error number might make sense, but I think that the use of the function is fairly limited, because lld doesn't try to do the exact same thing as MSVC link.exe. We sometimes emit a warning on which MSVC would emit a warning, but that's not too many.