[libcxx-commits] [libcxx] [libc++] Add a utility for checking the output of commands (PR #65917)

via libcxx-commits libcxx-commits at lists.llvm.org
Wed Sep 13 11:28:02 PDT 2023


EricWF wrote:

> > I've given a lot of thought to adding these sorts of tests before, but the inherent non-portability and limited utility have always kept me from doing it.
> > Could you more thoroughly explain the value added by these tests and why they can't be established in other ways?
> 
> We've wished that we would have access to `FileCheck` in our tests several times in the past, usually when we implemented optimizations. I don't understand the purpose of the `cmp_equal` tests & friends, but IMO the test for `libcxx/test/libcxx/algorithms/alg.modifying.operations/copy_move_codegen.sh.cpp` is really great. It makes sure that we lower to `memmove` under some circumstances, which we don't have any way to test otherwise. 

What does it mean if that test starts failing? I would argue "not much". 

We don't care that the functions call memmov, we care about the performance. If the compiler stops lowering to memmov, and instead lowers to faster instructions, do we care? No. Should we have a test that fails if such a change occurs? No.

What we're really trying to test is that we don't regress a benchmark. And that's tricky to do, so I understand the motivation to find another way to test that.

> I see a lot of value in being able to perform these checks in specific (rather unusual) circumstances. I also agree that it's not ideal to write the tool ourselves when we have FileCheck. I see two options for moving forward with this:
>
> 1. Either make FileCheck independent of the rest of LLVM so we can build it from scratch every time easily
> 2. Or use a pre-installed version of `FileCheck` in the Docker image. This will require making `FileCheck` support optional in the test suite, because we can't guarantee that folks will have installed it.

Cool, let's look into #1. 

But I'm pretty opposed to making the build+test iteration speed any slower by default. If we actually come up with a substantial amount of tests that need FileCheck, I would be happy to revisit it's necessity, but we're not there yet.

https://github.com/llvm/llvm-project/pull/65917


More information about the libcxx-commits mailing list