This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use the new .gen tests to implement __verbose_abort tests
ClosedPublic

Authored by ldionne on May 22 2023, 2:34 PM.

Details

Summary

This reduces the amount of boilerplate that we need to generate
for each commit. It also resolves a problem where the modular CI
would run extremely slow on this test because we'd define a macro
before including the standard library, defeating the module cache.

Diff Detail

Event Timeline

ldionne created this revision.May 22 2023, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2023, 2:34 PM
ldionne requested review of this revision.May 22 2023, 2:34 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 22 2023, 2:34 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne added inline comments.May 22 2023, 2:34 PM
libcxx/test/libcxx/lit.local.cfg
5 ↗(On Diff #524493)

This stuff would have to be duplicated until all the tests are moved to this new format.

What's the expectation for folks on Windows (where .sh isn't likely to work too well)?

What's the expectation for folks on Windows (where .sh isn't likely to work too well)?

The tests are generated using python scripts, so I don't think that's a problem.

libcxx/test/libcxx/assertions/headers_declare_verbose_abort.gen.py
27–32

I haven't tested this, but I think something like this would be easier to follow.

libcxx/test/libcxx/lit.local.cfg
5 ↗(On Diff #524493)

Could we move this to its own file? lit.local.cfg isn't exactly the first place I'd expect some data. Maybe just put something like header_test_info.py into libcxx/utils/data?

What's the expectation for folks on Windows (where .sh isn't likely to work too well)?

I believe we're using Lit's internal shell which actually also works on Windows. In this case, the invocation is extremely simple -- we basically call python <script> so that should work on windows just as well. There really isn't much of a change compared to how we used to do things, but this shifts the moment at which the generation happens (from being explicit via developer running a command to being implicit when you run the test suite).

What's the expectation for folks on Windows (where .sh isn't likely to work too well)?

I believe we're using Lit's internal shell which actually also works on Windows. In this case, the invocation is extremely simple -- we basically call python <script> so that should work on windows just as well. There really isn't much of a change compared to how we used to do things, but this shifts the moment at which the generation happens (from being explicit via developer running a command to being implicit when you run the test suite).

Ah, thank you! I hadn't realized this was using lit's internal shell, I thought these scripts were to be executed on the command line as well. If it's all going through python and lit, that sounds lovely to me. Thanks!

Thanks a lot for working on this! I really think this is very useful. Just a few nits, but I would like to see it after a green CI run.

libcxx/test/libcxx/lit.local.cfg
5 ↗(On Diff #524493)

+1

20 ↗(On Diff #524493)
ldionne marked 4 inline comments as done.May 25 2023, 8:00 AM
ldionne updated this revision to Diff 525618.May 25 2023, 8:01 AM

Rebase and address comments.

ldionne updated this revision to Diff 525620.May 25 2023, 8:01 AM
ldionne retitled this revision from [WIP][libc++] Support on-the-fly generated tests to [libc++] Use the new .gen tests to implement __verbose_abort tests.
ldionne edited the summary of this revision. (Show Details)

Update review description.

philnik accepted this revision as: philnik.May 25 2023, 9:33 AM

LGTM. Leaving final approval to @Mordante, since he had comments.

ldionne updated this revision to Diff 526080.May 26 2023, 8:38 AM

Rebase to fix CI

ldionne updated this revision to Diff 526448.May 29 2023, 8:53 AM

Make sure we exclude CMakeLists.txt from generated header tests

ldionne accepted this revision.May 29 2023, 12:28 PM
This revision is now accepted and ready to land.May 29 2023, 12:28 PM
libcxx/test/libcxx/assertions/headers_declare_verbose_abort.gen.py