zstring_view: opt-in zstring_view_safe variant, P3655R0-aligned substr, contains() polyfill

[mmthomas]

Summary

This PR adds a P3655R0-aligned safe variant of basic_zstring_view. A Traits template parameter selects between the existing trust-the-caller semantics and a new variant with runtime fail-fast on null inputs and a non-null data() invariant. Two aliases ship and both are usable in the same TU: wil::zstring_view keeps its existing trust-the-caller semantics, and wil::zstring_view_safe adds the safer behaviour. Smaller P3655R0-alignment items also land: the substr split and the contains() polyfill. A SAL annotation pass on the raw-pointer surface partially closes #278.

What ships

1. Traits-parameterised basic_zstring_view

template <class TChar, class Traits = std::char_traits<TChar>>
class basic_zstring_view : public std::basic_string_view<TChar, Traits> { ... };

// Legacy aliases
using zstring_view       = basic_zstring_view<char>;
using zwstring_view      = basic_zstring_view<wchar_t>;

// Safe aliases - non-null `data()` invariant + runtime fail-fast on nulls
using zstring_view_safe  = basic_zstring_view<char,    zstring_view_traits_safe<char>>;
using zwstring_view_safe = basic_zstring_view<wchar_t, zstring_view_traits_safe<wchar_t>>;

The dispatch is a zstring_view_traits_safe<TChar> traits type that derives from std::char_traits<TChar> and adds a nested empty_strings_are_non_null typedef; a SFINAE detector in wil::details (zsv_empty_is_nonnull_v<T>) picks up that typedef and branches the relevant ctors with if constexpr / enable_if.

The two variants are layout-compatible (both are { const TChar*, size_type } with no vtable); they differ only in the runtime checks that the safe variant interposes at construction.

2. (ptr, len) ctor: enforcement on the safe variant

The safe variant's (ptr, len) ctor routes the pointer through a private _require_non_null helper that calls WI_STL_FAIL_FAST_IF on null before the base ctor sees it. The body additionally guards the trailing-null read on a non-null pointer so a test-harness DoesCodeFailFast detour returns rather than really AVing. It's the same WI_STL_FAIL_FAST_IF primitive (and the same REQUIRE_ERROR-style testability) as the no-NULL-in-range fail-fast that both variants previously shipped.

The legacy variant's (ptr, len) ctor is unchanged: it keeps the same trust-the-caller semantics and the same latent UB on null input, so existing callers see no behaviour change.

3. Cross-variant conversion

• Safe to default is implicit. The safe variant's invariants are strictly stronger than the default variant's, so a safe-to-default conversion can never violate the target's invariants.
• Default to safe is explicit and runtime-checked. The default variant can hold data() == nullptr, so the conversion has to verify the source satisfies the safe invariant.
• Safe to std::basic_string_view<TChar> is implicit, and only present on the safe variant. The legacy variant gets this for free via the base class; the safe variant has to provide it explicitly because its base is std::basic_string_view<TChar, zstring_view_traits_safe<TChar>> (a different Traits).

Cross-variant ctors take their argument by const-reference rather than by value: the standard treats a ctor whose first parameter is the enclosing class type as a copy ctor and forbids the by-value form.

4. Why does the safe variant fail-fast on (nullptr, 0)?

The safe variant's (ptr, len) constructor fail-fasts on a nullptr pointer regardless of length. The C++23 stdlib's basic_string_view(nullptr, 0) is well-defined and produces data() == nullptr, size() == 0 ([string.view.cons]/4: [nullptr, nullptr) is a trivially valid empty range, and nullptr + 0 == nullptr per C++20 [expr.add]). The legacy wil::zstring_view inherits that behaviour.

The safe variant is intentionally stricter. The C++29 draft (P3655R0) specifies a stronger class invariant for basic_zstring_view in [zstring.view.general]: "In all cases, [data(), data() + size()] is a valid range and data() + size() points at an object with value charT()." (nullptr, 0) cannot satisfy that - data() + size() is nullptr and can't be dereferenced for the null terminator. P3655R0 leaves the invariant as a caller precondition (so violating it is UB) and notes that "all [non-(charT*) ctors] could be inadvertently misused."

wil::zstring_view_safe honors the same invariant as P3655R0 and enforces it with fail-fast at the precondition boundary - same contract as the draft, with diagnostics where the draft leaves UB. The choice of fail-fast (not throw) matches the existing no-NULL-in-range path previously shipped on the legacy wil::zstring_view: both ctors are noexcept, both diagnose precondition violations via WI_STL_FAIL_FAST_IF, and both surface as REQUIRE_ERROR in the test suite.

Callers who want the stdlib's nullable-maybe-empty semantics should use the legacy wil::zstring_view. Callers who want "empty intent" should use the default constructor wil::zstring_view_safe{}, which gives a dereferenceable empty buffer at zero runtime cost.

5. Other changes

• substr is split into two overloads with different return types. substr(pos) returns a basic_zstring_view (the tail of a null-terminated string is itself null-terminated); substr(pos, count) returns a std::basic_string_view (the slice isn't guaranteed null-terminated). This matches P3655R0.
• A contains() polyfill is added for callers building below C++23, gated on __cpp_lib_string_contains. The gate mirrors the existing __cpp_lib_format pattern in this file.
• A SAL annotation pass on the raw-pointer surface partially closes #278. The (ptr, length) ctor gets _In_reads_z_(stringLength + 1); the new contains() overload takes _In_z_; c_str() gets _Ret_maybenull_z_ (truthful for both variants).

The doc-block leads with the Traits split, then enumerates the safe variant's invariants, then calls out the slicing escape hatch that public inheritance leaves open (which P3655R0 closes in C++29 by rejecting inheritance outright).

Tests

Tests are parameterised so both variants exercise the same scenarios:

• A TEMPLATE_TEST_CASE matrix exercises both variants over {wil::zstring_view, wil::zwstring_view, wil::zstring_view_safe, wil::zwstring_view_safe} for construction, element access, starts_with / ends_with, contains, substr(pos) covariance, substr(pos, count) returning string_view, remove_prefix preserving null-termination, conversion to string_view, and constexpr usage. It branches on is_safe_zsv_v<ZSV> where the safe variant's invariants diverge.
• The hand-rolled per-char-type cases (TestZStringView, TestZWStringView, the literal block, and the std::format block) cover the legacy variant explicitly, and the templated suite gives the safe variant equivalent coverage.
• A separate TEST_CASE pins down the safe-variant-specific invariants: nullptr_t ctor deletion, (nullptr, 0) fail-fast at runtime, the non-null buffer on default-construct, and the implicit-to-string_view round-trip.

Divergence from P3655R0

The structural difference between WIL and P3655R0 is:

WIL's basic_zstring_view publicly inherits from std::basic_string_view. P3655R0 explicitly rejects inheritance, on the grounds that string_view's operator= can assign a non-null-terminated view onto a zstring_view instance and break the invariant. The proposal's reference implementation is a stand-alone class that holds data_ / size_ members directly.

WIL has shipped public inheritance since the class was introduced; dropping it would require reimplementing every basic_string_view member on the wrapper. The doc-block's @note acknowledges the slicing escape hatch and names P3655R0 as where the design is heading.

Other P3655R0 differences are about WIL's existing shipped public surface, not anything this PR changes:

• Type alias spelling. WIL ships zwstring_view; P3655R0 prefers wzstring_view.
• Additional character-type aliases. P3655R0 includes u8zstring_view / u16zstring_view / u32zstring_view; WIL doesn't.

Questions for maintainers

1. Are zstring_view_safe / zwstring_view_safe the right alias names? We considered safe_zstring_view and checked_zstring_view, and settled on the _safe suffix to keep the legacy aliases as the lexical lead.
2. Should we override data() to add _Ret_z_ on the safe variant? That would fully close #278 but would require hiding the inherited base method. c_str() is the documented null-terminated accessor and is annotated in this PR; we left data() inherited as the lower-risk choice. We're happy to add the override if you'd rather close wil::zstring_view data and c_str are not SAL annotated to indicate _Ret_z_, triggering analyzers #278 outright.

[mmthomas]

@jonwis — this is the WIL upstream you mentioned wanting. Putting it in your queue for a look when you have time.

[dmachaj]

I do wonder about the compat impact of this change. (Don't get me wrong, I think it is reasonable and probably better behavior to have a valid empty string for default construction). If there are any APIs that treat nullptr different than an empty string then this change will have side effects. Passing a default-constructed zstring_view will no longer be a nullptr missing parameter.

[mmthomas]

Worth thinking about what changes for existing code that does this:

if (zv.data()) {
    log_message(zv.c_str());
}

Under the previous behaviour, a default-constructed zv skipped the body. Under the new behaviour, the guard is always true on a default-constructed view, so log_message("") runs. The C runtime function (or whatever the body calls) sees an empty null-terminated string instead of not being invoked at all.

The observable effect at the call site depends on what the downstream function does with "":

• A logger emits an empty log entry instead of skipping the line.
• printf("%s", "") prints nothing.
• fopen("", "r") fails with ENOENT instead of the path-open being skipped.
• strlen("") returns 0.

So existing callers that were using nullptr as a sentinel to skip downstream work now invoke that work with an empty string. Within this repo the only call sites exercising the data() == nullptr pattern are the three test-line assertions in the diff. For production code that genuinely needs to distinguish "no string" from "empty string", the idiomatic replacement is std::optional<wil::zstring_view>.

Any recommendations on how to resolve the tension here? Happy to defer to your read on how WIL has handled similar trade-offs before.

[dmachaj]

I don't have any recommendations here. This is an intentional behavior change so it is all-or-nothing. Either this change is accepted and the behavior changes, or it isn't. I can't see a more-compatible middle ground.

@jonwis @dunhor any thoughts?

[jonwis]

I'm going to guess that there is existing code that relies on this behavior directly or indirectly. https://en.cppreference.com/cpp/string/basic_string_view/basic_string_view is documented as default-constructing to size==0 and data()==nullptr. My read of this overall change is trying to bring some better compat with the existing standards, so retaining the "it's nullptr" seems right. Nullptr as "not a value" is very common in the APIs that this type is designed to interoperate with.

[mmthomas]

Not sure I fully follow, @jonwis. The standard's proposed std::basic_zstring_view would change the behaviour you identified. It also does not derive from std::basic_string_view (perhaps for this reason, plus the slicing problem). What would you propose we do here?

Keep wil::basic_zstring_view aligned with the behaviour that std::basic_string_view brings (since it derives), or update it to match the better future?

Is there an opt-in or opt-out feature macro pattern we can use here that wil has used in the past to turn on the stricter/safer behaviour without breaking the data() == nullptr assumtion earlier code uses?

[mmthomas]

I found a pre-existing pattern in WIL to use to enable an opt-in approach that preserves existing caller assumptions regarding the data() value. The default behaviour is restored to = default; with data() == nullptr; callers that want the P3655R0 behaviour opt in by defining the new WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER macro before including wil/stl.h, which flips the default constructor to point at a static empty buffer.

The mechanism is WI_ODR_PRAGMA, the same approach used by WIL_KERNEL_MODE and WIL_EXCEPTION_MODE; LNK2038 fires on the tag ODR_violation_WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER_mismatch if two translation units disagree, and the developer can resolve the error by aligning either side with the other. The new tests/zsvsafe/ flavor mirrors tests/normal/ with the macro defined, so CI exercises both code paths.

[dunhor]

There seems to be a pre-existing assumption that this type is intended to implement/follow P3655R0 when in reality it was introduced independently to solve similar problems, and the two just naturally have different designs. In a vacuum I think I'd agree with the "always return a non-null string" design, however I agree with Jon and David's assessment that changing the behavior by default is too risky. I see three possible paths:

1. Use an ifdef (as you have now) to opt-in to the alternative behavior. Personally, I'm not a big fan of this. If you are not careful, you can easily run into annoying ODR issues. You work around this by using the pragma, however that comes with its own headaches. In particular, all libraries that get linked together need to agree. This is not a huge deal in smaller projects; however larger projects are basically prohibited from adopting the new behavior (e.g. the Windows source code). This is made worse by the fact that this mechanism of detecting ODR issues is at the include level - it does not consider whether or not a translation unit even uses the type. Overall, this is not too bad; such projects just wouldn't adopt the new behavior, which is equivalent to the new behavior having never been added, so I don't outright object to it.
2. Adopt the new behavior through the Traits template argument. E.g. something like a boolean Traits::empty_strings_maybe_null or Traits::c_str() that translates null pointers to empty string literals. Of course, a layer of indirection would be necessary to allow for something like char_traits which doesn't have this behavior. This would also require introducing Traits as a template argument, but that's probably fine. Defaulting to char_traits<TChar> would give the same behavior as before and likely wouldn't be a source breaking change for a majority of consumers. This would also allow multiple typedefs to co-exist next to each other that expose each behavior.
3. Introduce functions with stricter guarantees. This way the type can internally hold a null pointer, but if the caller needs a null terminated, non-null string, they can ask for one. E.g. (and these names are just me spitballing.. they are not necessarily well named) str.c_str_or_empty() and str.data_or_empty().

Personally, I like 2 the most, although a combination of 2 and 3 would be nice (allows consumers of the current type to more easily get a guaranteed-to-not-be-null string. That may be a bit overkill though...

[dunhor]

An additional complexity with option 2: the obvious thing would be to derive from std::basic_string_view<TChar, Traits>, however that would mean any attempts to use the traits type to enable this alternative behavior would cause the type to no longer decay to std::string_view/std::wstring_view, so there would need to be some rebind-like logic to get it to work as desired.

[mmthomas]

We went with option 2. The doc-block's "Cross-instantiation conversion" @note covers the rebind/overload-resolution caveat from the follow-up.

[dunhor]

Since the argument is constrained on is_convertible, I don't think _In_z_ is correct here. See: https://godbolt.org/z/bdzq9WT3s

[mmthomas]

Agreed on skipping the annotation here. Thanks for the godbolt example.

[dunhor]

Should (conditionally) be _Ret_maybenull_z_. Or maybe use _When_ to scope the conditions when it may be null, though I'm not sure if that's easily done here

[mmthomas]

Agreed, we looked at _When_ here too and didn't find a path forward that plays well with /analyze at the template level. Splitting c_str() into per-Traits overloads would also change its mangled name, which is an ABI hazard for any caller taking &basic_zstring_view::c_str. The annotation stays as a single _Ret_maybenull_z_, loose-but-correct for the safe variant.

[dunhor]

An alternative would be to introduce a (private) constructor that doesn't do this validation (e.g. via tag dispatch). That would reduce the complexity of this, and potentially other, functions

[mmthomas]

Thanks, the current revision takes that route. See the _trusted_tag ctor at L545-551 and substr() at L523-524.

[dunhor]

"safe" is a confusing name here. Would prefer something like "non-null"

[dunhor]

So the negative thing here is what I called out in my second comment - this will no longer decay to std::(w)string_view when using the non-null traits type. That's not going to break any existing code, so I don't object to it, but worth additional consideration

[dunhor]

My proposed solution. Update the non-null traits type to something like:

template <typename TChar, typename Traits = std::char_traits<TChar>>
struct nonnull_zstring_view_traits
{
    using char_traits = Traits;
    static constexpr bool empty_strings_are_non_null = true;
};

And then create a new traits type (a separate zsv_empty_is_nonnull is no longer necessary):

namespace details
{
    template <typename TChar, typename Traits>
    struct zstring_view_traits
    {
        template <typename T = Traits>
        static std::bool_constant<T::empty_strings_are_non_null> deduce_empty_strings_are_non_null(int);
        template <typename = Traits>
        static std::false_type deduce_empty_strings_are_non_null(...);

        template <typename T = Traits>
        static typename T::char_traits deduce_char_traits(int);
        template <typename T = Traits>
        static T deduce_char_traits(...);

        static constexpr bool empty_strings_are_non_null = decltype(deduce_empty_strings_are_non_null(0))::value;
        using char_traits = decltype(deduce_char_traits(0));
    };
}

And then update to derive from basic_string_view using this deduced traits type:

template <class TChar, class Traits = std::char_traits<TChar>>
class basic_zstring_view : public std::basic_string_view<TChar, typename details::zstring_view_traits<TChar, Traits>::char_traits>
{
    // Helper typedef to avoid needing to continuously deduce the traits type... 
    using BaseType = std::basic_string_view<TChar, typename details::zstring_view_traits<TChar, Traits>::char_traits>;

    // ...

Here's an example of the traits type working: https://godbolt.org/z/f5cshPG16

Edit: small changes because I copy/pasted too much... there's no need for the non-null traits type to derive from Traits any more, and fixed a minor issue with zstring_view_traits hard-coding char_traits

[dunhor]

Suggested change

	static constexpr const TChar* _require_non_null(const TChar* p) noexcept
	static constexpr const TChar* require_non_null(const TChar* p) noexcept

[dunhor]

Suggested change

	if (pStringData != nullptr && pStringData[stringLength] != 0)
	if (pStringData[stringLength] != 0)

_require_non_null will already fail fast if it's null

[dunhor]

You can simplify and merge these two constructors simply by baking the "should be non-null" check into the _require_non_null function (though I'd suggest renaming it then). Something like:

    static constexpr const TChar* check_pointer(const TChar* p) noexcept
    {
        if constexpr (details::zsv_empty_is_nonnull_v<Traits>)
        {
            if (p == nullptr)
            {
                WI_STL_FAIL_FAST_IF(true);
            }
        }
        return p;
    }

And then

    constexpr basic_zstring_view(_In_reads_z_(stringLength + 1) const TChar* pStringData, size_type stringLength) noexcept :
        std::basic_string_view<TChar, Traits>(check_pointer(pStringData), stringLength)
    {
        if (pStringData[stringLength] != 0)
        {
            WI_STL_FAIL_FAST_IF(true);
        }
    }

[dunhor]

Although... Since it reads pStringData unconditionally, the pointer can never be null under normal execution, so the conditional null check isn't even necessary...

[dunhor]

It would probably be more desirable if the Traits type matched as well.

[dunhor]

Although it's highly unlikely anyone ever uses something other than std::char_traits (and now the new one), I'd rather we not hard-code the types here. We could just accept a templated TraitsOther type and SFINAE on some is_compatible_v<Traits, TraitsOther> (exact implementation will depend on some other factors... ideally the check would actually be something more like is_same_v<MyBaseType, OtherBaseType> in a world where both derive from the same std::basic_string_view<CharT, std::char_traits<CharT>> or other custom traits type).

[dunhor]

If you go with my proposal, you'd basically gate on std::is_same_v<BaseType, typename basic_zstring_view<...>::BaseType> with possibly a friend thrown in there if necessary

[dunhor]

This is a hack to work around the fact that we don't "rebind" the traits type.

This would also mean that a static_cast to a reference type wouldn't work e.g. static_cast<std::string_view&>(str). Nor would implicit casts to functions accepting non-const references work

[dunhor]

Same thing, "safe" is misleading here... "non-null" is a much clearer name

[dunhor]

I'd suggest factoring out the non-null changes from the other improvements to make reviewing easier and so that those changes can be checked in quicker.