This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Associate each Symbol with an InputFile
ClosedPublic

Authored by int3 on Jan 8 2021, 9:01 AM.

Details

Reviewers
smeenai
Group Reviewers
Restricted Project
Commits
rG163dcd851309: [lld-macho] Associate each Symbol with an InputFile
Summary

This makes our error messages more informative. But the bigger motivation is for
LTO symbol resolution, which will be in an upcoming diff. The changes in this
one are largely mechanical.

Diff Detail

Event Timeline

int3 requested review of this revision.Jan 8 2021, 9:01 AM
int3 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 8 2021, 9:01 AM
int3 updated this revision to Diff 315441.Jan 8 2021, 10:22 AM

minor update

compnerd added inline comments.
lld/MachO/Symbols.h
202

Why not just cast<DylibFile>(file)? It will preserve the assertion and have the same behaviour.

217

Similar

int3 updated this revision to Diff 316179.Jan 12 2021, 11:42 AM

use cast<>

smeenai accepted this revision.Feb 2 2021, 3:37 PM
smeenai added a subscriber: smeenai.

LGTM

lld/MachO/SymbolTable.h
37–38

Pretty sure I've asked you this before (but I can't remember the answer): how come some parameters are named in the declaration and some aren't?

This revision is now accepted and ready to land.Feb 2 2021, 3:37 PM
int3 added inline comments.Feb 2 2021, 3:40 PM
lld/MachO/SymbolTable.h
37–38

my personal preference is to only include names if they convey information that's not already in the type name :) so StringRef name is useful, but InputSection *isec seems redundant

smeenai added inline comments.Feb 2 2021, 3:41 PM
lld/MachO/SymbolTable.h
37–38

Ah, right ... sounds good.

This revision was automatically updated to reflect the committed changes.

Hi @int3 , It looks like either this or https://reviews.llvm.org/D95265 is causing linking issues on Windows due to some MachO changes. Could I have you take a look?

Here's a buildbot result for reference:
http://lab.llvm.org:8011/#/builders/83/builds/3332

int3 added a comment.Feb 3 2021, 12:49 PM

thanks for the heads up. looking