Page MenuHomePhabricator

[tools][llvm-libtool] Emit warnings for files without symbols
ClosedPublic

Authored by alexander-shaposhnikov on Feb 1 2021, 6:43 PM.

Details

Summary
  1. Emit warnings for files without symbols.
  2. Add -no_warning_for_no_symbols (consistent with cctools libtool, see https://www.manpagez.com/man/1/libtool/osx-10.11.6.php)

Test plan: make check-all

Diff Detail

Event Timeline

alexander-shaposhnikov requested review of this revision.Feb 1 2021, 6:43 PM
alexander-shaposhnikov created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2021, 6:43 PM

Should we warn on files that would have been filtered out by -arch_only? (What does cctools libtool do?)

Should we warn on bitcode files that contain no symbols?

@smeenai : many thanks for the review,

  1. regarding bitcode files: cctools libtool doesn't warn on them
  2. regarding input files which are filtered out by -arch_only: cctools libtool doesn't warn on them either

Perhaps, it makes sense to add tests for these cases

In D95843#2540709, @alexshap wrote:

@smeenai : many thanks for the review,

  1. regarding bitcode files: cctools libtool doesn't warn on them
  2. regarding input files which are filtered out by -arch_only: cctools libtool doesn't warn on them either

Perhaps, it makes sense to add tests for these cases

Yup, we should definitely add tests.

We're currently checking for no symbols before we do the -arch_only filtering, right? So our current behavior diverges from cctools?

smeenai accepted this revision.Feb 3 2021, 5:42 PM

LGTM.

A few other cases that might be good to have test cases for as well, but I'll let you judge that:

  • Empty (and non-empty) archive members
  • Empty (and non-empty) universal binary members, possibly with and without -arch_only
This revision is now accepted and ready to land.Feb 3 2021, 5:42 PM

no_warning_for_no_symbols probably needs adding to the Comand Guide?

llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
94

Nit: other descriptions don't appear to end with a full stop.

260

Nit: don't end diagnostic messages with trailing full stop (as per style guide).

This revision was landed with ongoing or failed builds.Feb 16 2021, 5:53 PM
This revision was automatically updated to reflect the committed changes.