Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use _STL_INTERNAL_STATIC_ASSERT(false) when appropriate #4624

Merged
merged 10 commits into from
Apr 26, 2024

Conversation

StephanTLavavej
Copy link
Member

Followup to #4591.

This was prompted by VSO-2045281 "[RWC][prod/fe][Regression] Taichi and Root failed with error G030630B7: static assertion failed: Unexpected size", although it doesn't attempt to completely fix those failures in our Real World Code test suite. (It appears that these projects are using unsupported compilers - i.e. non-MSVC/Clang/CUDA/IntelliSense - to parse our STL headers, and these compilers haven't implemented CWG-2518.)

Some occurrences of static_assert(false, "message") are enforcing requirements for users, providing nice messages (ideally with Standard citations). However, many occurrences were checking for "can't happen" scenarios, i.e. damaged logic in the STL itself. We should use _STL_INTERNAL_STATIC_ASSERT for such checks, as this clearly distinguishes "user was a bad kitty" from "STL was a bad kitty". (Also, _STL_INTERNAL_STATIC_ASSERT is active only within our test suites, so it results in a microscopic throughput improvement for users.)

I reviewed all of the following occurrences (i.e. not an unthinking search-and-replace):

Location Change
Vectorized dispatchers _STL_INTERNAL_STATIC_ASSERT(false); // unexpected size
CPOs and views _STL_INTERNAL_STATIC_ASSERT(false); // unexpected strategy
byteswap() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected size
~_Mini_ptr() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected strategy
vector::_Construct_n() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected number of arguments
_Float_put_desired_precision() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected type; shouldn't be float
_Chrono_formatter::_Is_valid_type() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected type
chrono::clock_cast() _STL_INTERNAL_STATIC_ASSERT(false); // unexpected strategy

I performed a couple of targeted cleanups along the way (we try to avoid mixing major changes with cleanups, but these aren't entangled, and they are in the same area):

  • Cleanup static_assert messages and add citations in uses_allocator_construction_args().
    • I chose to keep the current structure, although having static_assert(condition) in one branch and static_assert(false) in the other isn't super consistent.
  • Style: Wrap tuple/variant "occur exactly once" messages consistently, to avoid wrapping static_assert(false.
    • I chose to keep static_assert(false) since it improves the error messages, even though other functions use static_assert(condition).

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Apr 23, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner April 23, 2024 23:30
@StephanTLavavej StephanTLavavej self-assigned this Apr 26, 2024
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej StephanTLavavej merged commit 0c245ad into microsoft:main Apr 26, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the static-assert-false-2 branch April 26, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants