This is an archive of the discontinued LLVM Phabricator instance.

lld/coff: Simplify error message for comdat selection mismatches
ClosedPublic

Authored by thakis on Feb 13 2019, 7:23 AM.

Details

Summary

Turns out nobody understands what "conflicting comdat type" is supposed to mean, so just emit a regular "duplicate symbol" error and move the comdat selection information into /verbose output.

This also fixes a problem where the error output would depend on the order of .obj files passed. Before this patch:

  • If passed one_only.obj discard.obj, lld-link would only err "conflicting comdat type"
  • If passed discard.obj one_only.obj, lld-link would err "conflicting comdat type" and then "duplicate symbol"

Now lld-link only errs "duplicate symbol" in both cases.

I considered adding a "Detail" parameter to reportDuplicate() that's printed in parens at the end of the "duplicate symbol" diag if present, and then put the comdat selection mismatch details there, but since users don't know what it's supposed to mean decided against it. I also considered special-casing the Detail message for one_only/discard mismatches, which in practice means "function defined as inline in TU 1 but as out-of-line in TU 2", but I wasn't sure how useful it is so I omitted that too.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Feb 13 2019, 7:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2019, 7:23 AM
ruiu added inline comments.Feb 13 2019, 1:08 PM
lld/COFF/InputFiles.cpp
502–505 ↗(On Diff #186654)

clang-format?

510 ↗(On Diff #186654)

I'd return early instead of doing this to effectively disable the following code, but you can't do that because we want to execute the code after the following switch. Maybe it's time to split this into a new function?

thakis updated this revision to Diff 186768.Feb 13 2019, 4:14 PM

pull out function

Extracted a function. Let me know if you want me to do the function extraction without other changes in a different CL, so that it's a behavior-preserving refactoring in one patch and a behavior change in a separate much smaller patch.

ruiu accepted this revision.Feb 13 2019, 4:18 PM

LGTM

lld/COFF/InputFiles.cpp
387 ↗(On Diff #186768)

Handle -> handle

This revision is now accepted and ready to land.Feb 13 2019, 4:18 PM
This revision was automatically updated to reflect the committed changes.
thakis marked an inline comment as done.Feb 13 2019, 7:16 PM