This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][test] - Merge 2 test cases together.
ClosedPublic

Authored by grimar on Nov 30 2020, 6:12 AM.

Details

Summary

This merges invalid-attr-section-size.test and invalid-attr-version.test
into invalid-attributes-sec.test.

This allows to have a single place where other related test cases can be added.

Diff Detail

Event Timeline

grimar created this revision.Nov 30 2020, 6:12 AM
grimar requested review of this revision.Nov 30 2020, 6:12 AM
MaskRay accepted this revision.Nov 30 2020, 2:08 PM

LG. I wonder the test should be named attributes-invalid.test since users can find related tests by checking the prefix.

This revision is now accepted and ready to land.Nov 30 2020, 2:08 PM

LG. I wonder the test should be named attributes-invalid.test since users can find related tests by checking the prefix.

I think renaming to attributes-invalid.test is a good idea. Will do.

jhenderson added inline comments.Dec 1 2020, 12:27 AM
llvm/test/tools/llvm-readobj/ELF/RISCV/invalid-attributes-sec.test
25–26

The intent of the invalid-attr-section-size.test was simply to show that llvm-readobj usefully reports the errors reported by the attribute parser, hence why the comment didn't mention the specific details. See https://reviews.llvm.org/D75833#1948304 and other comments in that review for context. Perhaps it is worth keeping the comment for that case generic like in the original, so that the intent isn't lost (the attribute parser unit tests are for testing the details of the parser).

grimar updated this revision to Diff 308575.Dec 1 2020, 1:26 AM
grimar marked an inline comment as done.
  • Addressed review comments.
jhenderson accepted this revision.Dec 1 2020, 1:36 AM

LGTM.

llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test
25 ↗(On Diff #308575)

I'd suggest this wording.

This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.