[llvm] r269784 - Change llvm-objdump, llvm-nm and llvm-size when reporting an object file error

[Justin Bogner via llvm-commits]

Kevin Enderby via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: enderby
> Date: Tue May 17 12:10:12 2016
> New Revision: 269784
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269784&view=rev
> Log:
> Change llvm-objdump, llvm-nm and llvm-size when reporting an object file error
> when the object is in an archive to use something like libx.a(foo.o) as part of
> the error message.
>
> Also changed llvm-objdump and llvm-size to be like llvm-nm and ignore non-object
> files in archives and not produce any error message.
>
> To do this Archive::Child::getAsBinary() was changed from ErrorOr<...> to
> Expected<...> then that was threaded up to its users.
>
> Converting this interface to Expected<> from ErrorOr<> does involve
> touching a number of places. To contain the changes for now the use of
> errorToErrorCode() is still used in one place yet to be fully converted.
>
> Again there some were bugs in the existing code that did not deal with the
> old ErrorOr<> return values.  So now with Expected<> since they must be
> checked and the error handled, I added a TODO and a comments for those.
 ...
> Modified: llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp?rev=269784&r1=269783&r2=269784&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp (original)
> +++ llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp Tue May 17 12:10:12 2016
> @@ -485,11 +485,17 @@ static void dumpArchive(const Archive *A
>    for (auto &ErrorOrChild : Arc->children()) {
>      error(ErrorOrChild.getError());
>      const Archive::Child &ArcC = *ErrorOrChild;
> -    ErrorOr<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
> -    if (std::error_code EC = ChildOrErr.getError()) {
> +    Expected<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
> +    if (!ChildOrErr) {
>        // Ignore non-object files.
> -      if (EC != object_error::invalid_file_type)
> -        reportError(Arc->getFileName(), EC.message());
> +      if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError())) {
> +        std::string Buf;
> +        raw_string_ostream OS(Buf);
> +        logAllUnhandledErrors(std::move(E), OS, "");
> +        OS.flush();
> +        reportError(Arc->getFileName(), Buf);
> +      }
> +      ChildOrErr.takeError();

I tried adding LLVM_NODISCARD to the Error type and noticed that we just
takeError here and don't inspect the result. Is this trying to ignore
the error (in which case I suppose it should consumeError) or did we
miss some handling here?

>        continue;
>      }