Page MenuHomePhabricator

LLD COFF: Add support for /force:multiple option
ClosedPublic

Authored by troughton on Aug 10 2018, 11:05 PM.

Details

Summary

This patch adds support for linking with multiple definitions to LLD's COFF driver, in line with link.exe's /force:multiple option.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

troughton created this revision.Aug 10 2018, 11:05 PM
troughton added a reviewer: ruiu.EditedAug 31 2018, 8:04 PM

Could you please either take a look or suggest a reviewer?

ruiu added inline comments.Sep 3 2018, 1:54 AM
COFF/SymbolTable.cpp
264

Then you don't need to change the function name.

329–336

Looks like this is the only place where you are using this function. I'd inline it.

troughton updated this revision to Diff 164542.Sep 7 2018, 4:52 PM

Address review comments by inlining errorOrWarnMultiple.

ruiu accepted this revision.Sep 10 2018, 8:10 AM

LGTM

COFF/SymbolTable.cpp
329

We don't use auto unless an actual type is obvious:

auto -> std::string

Variable names should start with an uppercase letter and should be concise:

message -> Msg
This revision is now accepted and ready to land.Sep 10 2018, 8:10 AM
troughton updated this revision to Diff 164760.Sep 10 2018, 3:37 PM
troughton marked 2 inline comments as done.

Specify std::string over auto and change message to Msg.

troughton accepted this revision.Sep 13 2018, 2:54 PM
troughton marked an inline comment as done.

If this looks fine, could you please merge it in? I’m not sure how the merge process is handled for LLVM.

ruiu added a comment.Sep 13 2018, 2:55 PM

If you have a commit access, you can do that yourself, but since you don't have one, I'll do that for you.

ruiu added a comment.Sep 13 2018, 2:58 PM

It needs a test, but I'll write that since the test should be small.

This revision was automatically updated to reflect the committed changes.