[llvm] r260488 - [readobj] Handle ELF files with no section table or with no program headers.

[Rafael Espíndola via llvm-commits]

>> I see the other way. You deleted error checking,
>
> I added a feature with tests that were missing some cases. I should
> have had the test. It does not require reverting the patch to fix
> this.

It is hard to see that all the previous error handling is in place if
it gets deleted as part of a large patch and then added again. It was
also hard to review a patch that deletes code it should not, deletes
test, includes refactoring and introduces a feature all at once.

The only reasonable way to make sure we get it right is to revert
(done), extract the refactoring bits (done) and then add the feature,
making sure the existing tests still pass.

>
>> introduced code that didn't conform to the style guide
>
> Where? dynamic_symbols()? I was following the style guide here:

The patch was got clang format clean. I honestly don't remember
exactly where, but noticed it when splitting the patch.

>> and the patch was not reviewed.
>
> I do not need pre-commit review to make changes to readobj.

For deleting code that I wrote? Seems bad practice.

> Per developer policy: Commits that violate these quality standards
> (e.g. are very broken) may be reverted. This is necessary when the
> change blocks other developers from making progress.

It *reverted* work that I had done, so you reverted code first. I got
it back to what it was. So yes, it was blocking progress. It was
blocking making sure we still had the features we had before you
decided you could just delete tests.

Cheers,
Rafael