Test P1502R1 Standard Library Header Units

[StephanTLavavej]

Works towards #60.

• stl/inc/memory_resource
  • Work around DevCom-1159869 "Standard Library Header Units: Bogus warning C4373 about virtual function overriding" by dropping const on these value parameters. This has been fixed in VS 2019 16.9 Preview 1.
• tests/std/test.lst
  • Add the new test.
• tests/std/tests/P1502R1_standard_library_header_units
  • __init__.py
    • Complicated stuff happening here.
  • custom_format.py
    • Build and run the test in the GitHub Python test harness. We iterate through a list of the stl_headers, building each header unit with /exportHeader, and recording the /headerUnit compiler option and object file path needed to consume it later. (This also contains code to handle /BE, which has to be ignored when building the header unit. /BE is not yet active.)
  • custombuild.pl
    • Build and run the test in the MSVC-internal Perl test harness. I haven't written code to handle /BE yet. This needs a new environment variable STL_INCLUDE_DIR (added in MSVC-internal distrib.yaml) which is the single directory where STL headers live.
    • Because the compiler's implementation of modules is quickly moving, and we have two different test harnesses, I'm doing something special here. The Perl test harness will compile with /DMSVC_INTERNAL_TESTING, allowing us to activate portions of the test that successfully compile and run with the latest development build, before those fixes ship in the public VS Preview. As the TRANSITION comments note, this is temporary - after things settle down, we'll be able to remove this macro.
  • env.lst
    • This test is special, so it has its own configurations. (For example, modules require /Zc:preprocessor.) The apparently-strange pattern of RUNALL_CROSSLISTs is imitating how the usual_matrix family works.
    • This test takes a long time to compile (building each header unit sequentially), so I'm currently testing only the 4 main combinations of release/debug and dynamic/static. /analyze works but is much slower; as it found no compiler bugs, I have disabled it for now. /BE is blocked by Microsoft-internal VSO-1232145 "EDG ICEs when consuming Standard Library Header Units".
  • lit.local.cfg
    • I carefully copy-paste-modified this file and aligned its whitespace.
  • test.cpp
    • Test the entire STL. 😹
    • The intent of this test is to import every STL header as a header unit, and then minimally exercise its compile-time and/or run-time functionality. This has found many compiler bugs that would block users; see P1502R1 Standard Library Header Units #60 for the list.
    • This does not attempt to exercise every class or function in the STL.

Adding this test to GitHub and MSVC now will prevent further regressions as we continue to change the STL and @cdacamar continues to fix the compiler. When all of the major blocking compiler bugs are fixed, we'll be able to declare feature-completeness.

Microsoft-internal mirror: MSVC-PR-282732

[MikeGitb]

test.cpp

Have you considered to check each import in a separate TU in addition to importing them all into a common file?
Of course that could be done in a separate PR.

[StephanTLavavej]

@MikeGitb

Have you considered to check each import in a separate TU in addition to importing them all into a common file? Of course that could be done in a separate PR.

That could be done, but it would take longer to build (~2x, I think), it would be more work to maintain and keep in sync, and I am unsure whether it would find any compiler bugs that the megatest isn't already finding. From my understanding of what @cdacamar explained to me, importing all header units is the most stressful case, as the compiler has to match up declarations.

@CaseyCarter

Aside: diagnosing failures of this megatest is not fun.

Yes, welcome to my life 😿

Looking at only x64, there are three compile failures (more occurrences of DevCom-1224512?):

Thanks for investigating, and sorry about that. I had iterated on this until I had gotten the test passing with both MSVC-internal prod/fe and GitHub, when compiled by hand for x86 /MD. Naturally, I now see that it's the only configuration out of all 8 that are passing on GitHub. (The MSVC-internal tests are also failing - despite the fact that a "local" run of x86 passed for me last night.)

I'll fix this up and push a revision.

and one runtime failure - looks like a crash - after Testing <fstream>.

Yep, that's why those puts() lines are there. I had been battling the crashes earlier, and I thought they were all gone. Nope!

[CaseyCarter]

Aside: diagnosing failures of this megatest is not fun.

Yes, welcome to my life 😿

It wouldn't come close to being worth the work to split it up, but it would be really nice if this were a single test per-header instead. We'd at least get a line or two of context in the logs. (Hindsight is 20/20, etc. etc.)

[StephanTLavavej]

@CaseyCarter

It wouldn't come close to being worth the work to split it up, but it would be really nice if this were a single test per-header instead. We'd at least get a line or two of context in the logs. (Hindsight is 20/20, etc. etc.)

See @MikeGitb's question - smaller tests would be easier to diagnose, but the larger test is both more realistic and (as I understand it) more stressful for the compiler, so it is a more effective test.

My understanding is that classic #include is basically neutral on whether just the necessary headers are included, or all of them - obviously including more headers will expand overload sets etc., but the nature of #pragma once/include guards means that there is no actual duplication of functionality. In contrast, each header unit has to independently drag in machinery, so the compiler has to "deduplicate" identical definitions. Several of the bugs I've reported occur only when the right mix of header units are imported.

[CaseyCarter]

@CaseyCarter

It wouldn't come close to being worth the work to split it up, but it would be really nice if this were a single test per-header instead. We'd at least get a line or two of context in the logs. (Hindsight is 20/20, etc. etc.)

See @MikeGitb's question - smaller tests would be easier to diagnose, but the larger test is both more realistic and (as I understand it) more stressful for the compiler, so it is a more effective test.

I'm really just griping about needing to download test logs to analyze failures; my concerns would be addressed by removing the puts before each test case. That said, I still won't ask for a change because I think it wouldn't have occurred to me to complain if the tests were passing and I hadn't looked at the logs.

[StephanTLavavej]

Oh, the puts are interacting with previews of truncated logs? I see, thanks for explaining. Unfortunately I think we need to keep them due to runtime failures (I added them to diagnose where crashes were happening). While downloading logs is annoying, I have found that locally iterating on tests is necessary, and that displays the entire context.

[MikeGitb]

@StephanTLavavej

That could be done, but it would take longer to build (~2x, I think), it would be more work to maintain and keep in sync, and I am unsure whether it would find any compiler bugs that the megatest isn't already finding.

My concern was mostly around the question if a individual import actually provides the things it should, but I don't have any real reason to assume that there is a problem, so I guess you are right, that there isn't a real benefit to it.

[StephanTLavavej]

@MikeGitb Ah, I see - that is a very valid concern, and applies to classic includes too. We have an “include each header alone” test that’s very good at finding headers that can’t even be included by themselves - but it doesn’t test that the header provides what it should. It’s often pretty difficult to use a header without including others that would make the test less specific.

I think that libcxx’s fine-grained tests probably have the best combination of having fairly minimal includes combined with checking that everything is being provided - you’re absolutely right that our std test suite is missing such coverage.