microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
9.88k stars 1.45k forks source link

Enable `variant` P0608R3 in C++17, update LLVM, overhaul `variant`/`any`/`optional` tests #4713

Closed StephanTLavavej closed 1 week ago

StephanTLavavej commented 3 weeks ago

Overview

This updates <variant> to make a C++20 behavioral change unconditional, updates our LLVM submodule yet again to pick up small improvements, and finally performs a large overhaul of our LLVM-derived tests for variant, any, and optional. This overhaul picks up newly-added test coverage (e.g. for spaceship comparisons), cleans up test coverage that was unintentionally missing, and splits the extremely large variant test into LLVM-derived and MSVC-specific parts.

Product code

I originally worried about the source-breaking impact of extending this C++20 behavioral change down to C++17. However, libc++ and libstdc++ have implemented this downlevel, the ecosystem has had some time to adapt (for projects that are portable, or compiled with MSVC C++20), and I think the remaining cleanup costs will be manageable.

I thought about guarding this with an escape hatch macro, but I'd like to try just ripping off the bandage. Unlike (for example) header-inclusion escape hatches, supporting both the old and new modes would require the old mode to be tested, and test coverage is what finally pushed me to make this change (as libc++'s tests started assuming this new behavior).

LLVM

transform_llvm.sh

P0088R3_variant

P0220R1_any

P0220R1_optional

Escape hatches

P0088R3_variant_msvc

x64 /std:c++latest compiler memory consumption as reported by /d1reportMemorySummary:

Test Flavor Before After
P0088R3_variant Plain 3.1 GB 2.1 GB
P0088R3_variant_msvc Plain N/A 1.1 GB
P0088R3_variant /analyze 4.9 GB 3.4 GB
P0088R3_variant_msvc /analyze N/A 1.6 GB

This improved total test runtime from 72.5s to 56.4s (1.29x speedup). Presumably this was caused by better parallelism (splitting up long-tail /analyze configurations) and won't significantly impact full test runs.

StephanTLavavej commented 2 weeks ago

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.