Page MenuHomePhabricator

[LLD] [COFF] Wrap file location pair<StringRef,int> in Optional<>. NFC.
ClosedPublic

Authored by mstorsjo on Oct 14 2019, 12:14 AM.

Diff Detail

Event Timeline

mstorsjo created this revision.Oct 14 2019, 12:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 14 2019, 12:14 AM
MaskRay added a comment.EditedOct 14 2019, 7:09 AM

I am in favor of this change. It makes the error case clearer.

lld/COFF/PDB.cpp
1818

return {{filename, *lineNumber}}; works (but I am not sure whether others like it)

mstorsjo marked an inline comment as done.Oct 14 2019, 11:10 AM
mstorsjo added inline comments.
lld/COFF/PDB.cpp
1818

Ah, that looks totally fine to me, and is nicely compact. std::make_pair() works as well, allowing omitting the template parameters.

rnk added inline comments.Oct 14 2019, 2:20 PM
lld/COFF/PDB.cpp
1818

I like either make_pair or double braces, as long we don't have to write the template arguments.

mstorsjo updated this revision to Diff 224957.Oct 14 2019, 11:17 PM
mstorsjo edited the summary of this revision. (Show Details)

Updated to use std::make_pair, which makes it consistent with lld/ELF.

ruiu accepted this revision.Oct 15 2019, 2:10 AM

LGTM

This revision is now accepted and ready to land.Oct 15 2019, 2:10 AM
This revision was automatically updated to reflect the committed changes.