Improve debug checks (prelude to STL Hardening)

[StephanTLavavej]

🗺️ Overview

Works towards #5090. For ease of review, this PR contains all of the cleanups and enhancements that I noticed along the way. The following PR will then be able to focus on removing _CONTAINER_DEBUG_LEVEL and adding STL Hardening without mixing in worthwhile-but-distracting changes.

Notably, this PR has audited the STL for "Continue on Error" (CoE) compatibility, allowing users to define _STL_CRT_SECURE_INVALID_PARAMETER to something that returns. (Throwing an exception is still super ultra forbidden.) Automated test coverage for CoE may follow in a future PR (it would inherently involve UB, so I need to investigate).

This also improves debug codegen for the STL's debug checks, which were emitting unnecessary branches.

💬 Comment cleanups

• Remove CDL preprocessor comments after a single line.
• Remove CDL preprocessor comments after a single _STL_VERIFY over multiple lines.

In general, we comment our #else and #endif directives to clarify where "regions" begin and end. However, for very short regions that aren't densely nested in other preprocessor directives, we're willing to omit the commits as they cause more clutter than clarity.

Here, removing the comments will make it easier to edit the conditions later. (All CDL conditions will be replaced with either the STL Hardening pattern, or IDL != 0.)

📜 Documentation cleanups

• docs/import_library.md: Don't mention CDL - it doesn't affect ABI, and will be removed.

📄 Debug message improvements

• Improve debug messages: "array<T, 0> subscript is invalid"
  • This identifies that we're dealing with array<T, 0>, and that the call is permanently bogus, following the example of "array<T, 0>::front() is invalid".
• Improve debug messages: "MEOW subscript out of range"
  • This is consistent with vector.
  • mdspan's message was very opaque; starting with "mdspan subscript out of range" provides clarity. Additionally, "I" was previously used without being defined; I've replaced it with its definition from /2.
  • ranges::view_interface now refers to itself with qualification.
• Improve debug messages: "MEOW() called on empty WOOF"
  • deque now mentions whether pop_front() or pop_back() was called.
  • optional now mentions whether operator*() or operator->() was called.
  • ranges::view_interface now refers to itself with qualification.
• Improve debug messages: Clarify span's first<Count>(), last<Count>(), subspan<Offset, Count>().
• Improve debug messages: Clarify string_view remove_prefix(), remove_suffix().
  • Spell the function names literally for improved searchability. Mention the short version of the class name too. Adjust "longer" to "larger" for style.
• Improve debug messages: std::expected.
  • This splits "expected stores an error, not a value" into:
    • "operator->() called on a std::expected that contains an error, not a value"
    • "operator*() called on a std::expected that contains an error, not a value"
  • And changes "expected stores a value, not an error" into:
    • "error() called on a std::expected that contains a value, not an error"
  • This mentions which member function is being called. Because "expected" doesn't really sound like a noun, I've added "std::" qualification and made the phrases less terse. And as @sivadeilra suggested, "stores" has connotations of "memory writes", so "contains" is clearer.
• Improve debug messages: Improve grammar and clarity with "basic_format_arg contains an impossible type".
  • In general, I limited the scope of these debug message improvements to those that will be shared with STL Hardening checks. However, I noticed this one in <format>'s implementation and couldn't resist fixing it.

☑️ Test cleanups

• Test cleanup: Remove CDL definitions from some <ranges> tests.
  • These weren't CDL-sensitive at all, and were properly guarded with IDL != 0.
• Test cleanup: Drop CDL from <format> test, properly test negative dynamic width/precision.
  • <format> isn't CDL-sensitive.
  • Negative dynamic width/precision are required to throw format_error; we don't use "death tests" for exceptions. For clarity and consistency, add positive and negative test cases for both. This may slightly duplicate existing coverage, but negative dynamic precision wasn't being properly tested AFAICT.
• Test cleanup: Remove VSO-847348 workarounds.
  • VSO-847348 "c1xx ICEs deducing array size inside a temploid" was fixed at some point during the last 6 years (verified with prod/fe x86chk).
• Test cleanup: Use if constexpr instead of tag dispatch and plain if.

🪄 Code cleanups

• Code cleanups: Add const, unwrap "strengthened".
• Code cleanup: Conditional _STL_REPORT_ERROR => _STL_VERIFY or _STL_ASSERT
  • In <memory_resource>, we always call _Prepare_oversized, but the _STL_REPORT_ERROR was guarded by #ifdef _DEBUG. Convert this to _STL_ASSERT, which is consistent with the _STL_ASSERT that appears immediately below.
  • In <numeric> gcd(), we're using De Morgan's laws, the STL maintainer's best friend.
  • In <numeric> lcm(), extract const bool _Overflow so the _STL_VERIFY has no side effects.
• Code cleanup: _STL_VERIFY(false) => _STL_REPORT_ERROR
  • In <mdspan>, add return 1; in stride() to support upcoming changes for continue-on-error.
  • In <regex>, unconditionally return false; for the same reason.
• Code cleanup: Consistently order case before default.
• Code cleanup: Fuse _Check_alignment into atomic_ref ctor.
• Code cleanup: Collapse _ATOMIC_REF_CHECK_ALIGNMENT into _STL_ASSERT.
  • This doesn't change behavior (it remains a debug-only check), it just removes an escape hatch that was never used.
• Code cleanup: Avoid duplicating _Off and _Count. Add const to _Moved.
• Code cleanup: Avoid duplicating _STD copy() call.
• Code cleanup: Avoid more duplication.
  • We can always extract _Newsize for clarity.
• Code cleanup: IDL == 2 implies debug, so _STL_ASSERT should be _STL_VERIFY.
  • This is especially noticeable when _STL_VERIFY already follows.
• Code cleanup: Extract _Can_memcpy for clarity, simplifying following CoE fix.
• Code cleanup: Use _Check_MEOW_memory_order to avoid some _FALLTHROUGH in <atomic>.
  • For _ATOMIC_LOAD_ARM64 and _ATOMIC_POST_LOAD_BARRIER_AS_NEEDED, use _Check_load_memory_order and arrange the control flow to preserve the fallthrough behavior.
  • Because _Check_load_memory_order takes a real enum, we need to compare against the enumerators and avoid static_cast<unsigned int>(_Order).
  • For _ATOMIC_STORE_PREFIX, expand this absolutely hideous macro 🤮 at each point of usage, then perform a similar transformation. As a bonus, we get to drop a bunch of early return;s.

🚀 Enhancements

• Enhancement (for CoE): Adjust uninitialized_meow's logic.
  • We'll continue right into an infinite loop, but at least we won't fall off the end of a non-void function.
• Enhancement (for CoE): After _INVALID_MEMORY_ORDER, consistently fallthrough to seq_cst.
  • We were already doing this in other places.
• Enhancement: _STL_ASSERT => _STL_VERIFY for checks within IDL >= 1.
  • We don't really care about IDL exactly equal to 1, but these should clearly be checked in IDL=1 release, consistent with the following checks. Note that we already had an _STL_VERIFY below.
• Enhancement: Replace _STL_ASSERT with _STL_INTERNAL_STATIC_ASSERT.
  • These conditions are already guaranteed by duration and ratio. They don't need to be checked for users, especially at runtime.
• Enhancement: Remove IDL=2 null asserts from auto_ptr deref/arrow.
  • It's silly to assert that _Myptr is non-null when we're about to dereference it (or return it for imminent dereferencing). We purged this category of asserts from the STL aeons ago, but must have missed auto_ptr. The idea is that null dereferences are just as obvious in the debugger. Note that this makes the code consistent with unique_ptr and shared_ptr.
• Enhancement: _Span_iterator::operator-= was unconditionally checking.
  • First, a cleanup: _Verify_offset does something for IDL >= 1 only, so we can guard the callsite in operator+=.
  • In operator-=, the pathological "integer overflow" check was being performed unconditionally. Guard it for IDL >= 1, and the following _Verify_offset call.
• Enhancement: Improve _STL_REPORT_ERROR, _STL_VERIFY codegen.
  • See: https://godbolt.org/z/b95sjPPGo
  • And:

    STL/stl/inc/atomic

    Lines 103 to 105 in fc15609

    	// note: these macros are _not_ always safe to use with a trailing semicolon,
    	// we avoid wrapping them in do {} while (0) because MSVC generates code for such loops
    	// in debug mode.

  • do { ... } while (false) has debug codegen costs. We don't need this because we ALWAYS use braces for control flow.
  • if (cond) { } else { ... } also has debug codegen costs. We control the conditions, and the Standard's specification for boolean-testable has improved, so we shouldn't need this.
  • We can drop -Wassume ("assumption is ignored because it contains (potential) side-effects") suppression for Clang in _STL_VERIFY. Clang doesn't support /analyze, but VSO_0938757_attribute_order is special - it runs Clang with /D_PREFAST_ to verify that we're ordering standard attributes before non-standard attributes. Because this is the only test that's activating _Analysis_assume_ for Clang, compiling it with -Wno-assume is better than modifying the product code.

[davidmrdavid]

I've completed a first pass. Thanks a lot, again, for the pristine commit history, it made this a lot easier. I've left some comments, most are non-blockers and others reveal bits I don't fully grok yet. I'll look to resolve these by tomorrow so I can unblock you sooner rather than later.

Excited for hardening! 💎 💎 💎

[StephanTLavavej]

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