Page MenuHomePhabricator

[FileCheck] Add --check-prefixes as a shorthand for multiple --check-prefix options.
ClosedPublic

Authored by dsanders on Jun 13 2016, 8:18 AM.

Details

Summary

This new alias takes a comma separated list of prefixes which allows
'--check-prefix=A --check-prefix=B --check-prefix=C' to be written as
'--check-prefixes=A,B,C'.

Depends on D21292 since it reduces the line length of elf_header.s.

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders updated this revision to Diff 60530.Jun 13 2016, 8:18 AM
dsanders retitled this revision from to [FileCheck] Add --check-prefixes as a shorthand for multiple --check-prefix options..
dsanders updated this object.
dsanders added a subscriber: llvm-commits.

Wow. I am blown away by the sophisticated implementation. :-)

So, the MC/Mips test ought to be committed separately (as part of D21292 clearly).
Also I think you went a bit overboard with the FileCheck tests, no need to replicate *all* the multi-prefix runs. I'd think a couple representative tests would be enough, maybe just check-multiple-prefixes-mixed.txt and multiple-missing-prefixes.txt.

With that, LGTM.

Wow. I am blown away by the sophisticated implementation. :-)

I didn't believe it could be that simple.

So, the MC/Mips test ought to be committed separately (as part of D21292 clearly).

Yep, it's in here because of the order of the patches in my current series but it looks like this will be committed first so I'll fold that part into D21292.

Also I think you went a bit overboard with the FileCheck tests, no need to replicate *all* the multi-prefix runs. I'd think a couple representative tests would be enough, maybe just check-multiple-prefixes-mixed.txt and multiple-missing-prefixes.txt.

Makes sense. In addition to the two you mention, I'll also keep the two *-no-match*.txt tests. Most of the tests can falsely pass if only one prefix takes effect so the *-no-match*.txt tests are needed to make sure all the prefixes are being processed.

With that, LGTM.

Thanks

dsanders updated this revision to Diff 60683.Jun 14 2016, 7:25 AM

Remove elf_header.s it will be folded into D21292.
Remove unnecessary tests.

This revision was automatically updated to reflect the committed changes.

Doh! Forgot to say... please document this new option (docs/CommandGuide/FileCheck.rst).

Added documentation in r272683.