[PATCH] D78589: [lit] Add an option to print all features used in tests

Thu May 28 11:33:24 PDT 2020

ldionne added a comment.

Sorry for the delay in replying to this -- I got pulled into other things.

In D78589#2002960 <https://reviews.llvm.org/D78589#2002960>, @yln wrote:

> Thanks for being thorough and explaining your reasoning so well.  I now agree that "Unavailable Features" introduces too much noise to simply add it to the existing option without the user asking for it.
>
> Two more quick questions:
>
> 1. We present features as a sum of all used features, however they are specific to configs/test suites. Could we group them by test suite? Maybe a good way to do this would be to extend the print_discovered function. This way we can also combine output from the  "show and exit" options.

I actually see the features as being whatever features are used in the test files discovered by Lit -- not as a per test-suite thing. I actually don't think it makes a lot of sense to run Lit with multiple test suites at once (for example `--param` are shared across all test suites which is weird), but that's a different story.

> 2. Currently we have "Available Features" in `--show-suites`, which can be either used or unused.  When the user also asks for `--show-used-features` then maybe it would still be helpful to somehow mark the unused ones without introducing too much visual noise (e.g., with an asterisk + footnote).  This would help with "de-crufting" in at least one direction.

When we use `--show-used-features`, `"Available Features"` are not printed. Are you saying when one uses both `--show-suites` and `--show-used-features`? IMO, that's adding complexity for fairly little benefit -- I doubt anyone would even notice that feature. I like thinking that Lit can be used as a swiss army knife for extracting information about a test suite (or a bunch of tests), and then combined with a bit of Bash scripting or whatever to obtain the desired information.

Unless you feel strongly about points above, I'd like to move forward with this to avoid rotting of this patch.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78589/new/

https://reviews.llvm.org/D78589