Page MenuHomePhabricator

[llvm-readelf] Print "File: lib.a(file.o)" info when dumping archive files.

Authored by ychen on Jul 8 2019, 12:38 PM.

Diff Detail


Event Timeline

ychen created this revision.Jul 8 2019, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2019, 12:38 PM
MaskRay added inline comments.Jul 8 2019, 6:39 PM
151 ↗(On Diff #208479)

There was one empty line between } and File: , now there are two, due to the Writer.startLine() << "\n"; change.

604 ↗(On Diff #208479)

This will give us a trailing empty line. I think in a few other places, we do:

if (not first)
  print a new line

This pattern will not have a trailing empty line.

I think these probably don't matter but it'd be great if we can be consistent in the future..

ychen updated this revision to Diff 208572.Jul 8 2019, 7:13 PM
ychen marked an inline comment as done.
  • update
ychen marked 2 inline comments as done.Jul 8 2019, 7:16 PM

Thank you for capturing that. The added new line in the previous revision is for GNU output. I should have done what LLVM output did: add a prefixing new line.

MaskRay added inline comments.Jul 8 2019, 9:06 PM
167 ↗(On Diff #208572)

Add a MULTIHEADER-ARCHIVE-EMPTY: and change this line to:
to capture the newline issue.

ychen updated this revision to Diff 208589.Jul 8 2019, 9:19 PM
  • update
ychen marked an inline comment as done.Jul 8 2019, 9:19 PM
MaskRay accepted this revision.Jul 8 2019, 9:52 PM
MaskRay added inline comments.
167 ↗(On Diff #208572)

For consistency other File: should receive the same treatment..

This revision is now accepted and ready to land.Jul 8 2019, 9:52 PM
grimar accepted this revision.Jul 9 2019, 12:25 AM


Looks good aside from a couple of minor points.

5 ↗(On Diff #208589)

Nit: I don't think it matters too much, but -D is a single-letter option, so would normally only have one dash before it.

8 ↗(On Diff #208589)

I think in this test particularly, you should check that there are the right number of new lines before and after "File: ..." using appropriate CHECK-NEXT/EMPTY markers.

11 ↗(On Diff #208589)


ychen updated this revision to Diff 208821.Jul 9 2019, 2:49 PM
ychen marked 4 inline comments as done.
  • update
This revision was automatically updated to reflect the committed changes.