[PATCH] D13989: This removes the eating of the error in Archive::Child::getSize() when the charactersin the size field in the archive header for the member is not a number.

[Rafael Espíndola via llvm-commits]

> I would say it depends on the tool and what the tool is doing.
>
> In the case above the llvm-ar tool is printing out the archive header.  Since I often run tools on malformed files during development I like tools to show be what is there rather than just calling “fail” and bailing out in my opinion.  In this case what I really want is to see the characters of the archive header’s field for Size as during development I likely messed things up and need to see what is there so I can fix things.
>
> But to do this I would need to use the getHeader() method then print the Size[10] characters in this case.  And this really should be done for all the fields.  Which would mean making all the methods like getUID(), getGID(), etc, all return ErrorOr<> like getSize().  But that seams like changes that should be left for another patch.

Maybe. But we should not be changing behavior in this patch. And when
adding better error messages, it should still be done by calling fail.
It is important that we get a non zero error code and don't complicate
the code trying to handle corrupt files.

> I agree with you on that but I think the two places that needed this change need to deal better with errors.  As they are in the Jit and should not call report_fatal_error() with malformed input.  But I think it best for Lang or others to clean up the error handling and work to eliminate the use of report_fatal_error() for malformed input.

Yes. Before your first patch this was getting to llvm_unreachable, so
it is an improvement.

Taking a look at the new patch.

Cheers,
Rafael