Skip to content

basic_zstring_view: add opt-in nonnull variants - #668

Draft
Monroe Thomas (mmthomas) wants to merge 6 commits into
microsoft:masterfrom
mmthomas:users/mmthomas/nonnull-zstring-view
Draft

basic_zstring_view: add opt-in nonnull variants#668
Monroe Thomas (mmthomas) wants to merge 6 commits into
microsoft:masterfrom
mmthomas:users/mmthomas/nonnull-zstring-view

Conversation

@mmthomas

@mmthomas Monroe Thomas (mmthomas) commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

wil::zstring_view is a non-owning view of a null-terminated string. Its default constructor follows std::string_view: the view is empty and data() is null. That behavior must remain unchanged because existing callers may use null to mean "no string."

Some callers instead need an empty view that can be passed directly to a C API without first checking for null. This PR adds opt-in nonnull_zstring_view and nonnull_zwstring_view aliases for that use case. Their constructors reject null pointers, and their default constructors point at an internal empty string.

wil::zstring_view nullable;            // data() == nullptr
wil::nonnull_zstring_view nonnull;     // data() != nullptr, c_str()[0] == '\0'

printf("%s", nonnull.c_str());         // safe without a null check

The existing zstring_view and zwstring_view aliases retain their current behavior.

C++ standardization

WG21 is standardizing the same general abstraction for C++29 as std::basic_cstring_view in P3655R5, std::cstring_view. The proposed type is a non-owning view of a null-terminated string. Its default constructor refers to a static null terminator, so data() and c_str() return a valid empty string rather than null, and direct nullptr construction is deleted.

P3655 is an active proposal rather than part of the published C++ standard. WIL retains its existing zstring_view naming and public-inheritance design; this PR adds an opt-in construction policy that provides the proposal's non-null empty-state behavior without changing existing callers.

Public API

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;
};

template <class TChar, class Traits = std::char_traits<TChar>>
class basic_zstring_view;

using nonnull_zstring_view =
    basic_zstring_view<char, nonnull_zstring_view_traits<char>>;
using nonnull_zwstring_view =
    basic_zstring_view<wchar_t, nonnull_zstring_view_traits<wchar_t>>;

The policy type keeps the non-null behavior separate from the character traits used by std::basic_string_view. As a result, nonnull_zstring_view and zstring_view both derive from std::string_view, rather than deriving from different std::basic_string_view specializations.

This matters for normal C++ interoperability:

wil::nonnull_zstring_view value{"hello"};
std::string_view& base = value; // binds to the inherited base object

Custom character traits remain supported through nonnull_zstring_view_traits<TChar, Traits>.

Construction and conversion behavior

  • Default construction produces an empty view backed by an internal null terminator.
  • Direct nullptr construction is deleted.
  • Other null pointer inputs report a contract violation through WIL's existing fail-fast mechanism.
  • String literals, std::basic_string, compatible string-like objects, and valid pointer inputs behave like the existing type.
  • Conversion from a non-null view to the existing nullable view is implicit because it weakens the guarantee.
  • Conversion from a nullable view to a non-null view is explicit and checks the source pointer.
  • substr(pos) preserves the selected policy. A substring of a default-constructed non-null view therefore remains non-null.
  • Object size and trivial copyability are unchanged.

Inheritance limitation

basic_zstring_view publicly inherits from std::basic_string_view. This permits a caller to explicitly obtain a mutable base reference and assign a nullable base view:

wil::nonnull_zstring_view value{"hello"};
std::string_view& base = value;
base = std::string_view{}; // bypasses the non-null construction policy

The new type enforces non-null construction through its own API. In debug builds, the derived c_str() asserts if base-class mutation has changed the stored pointer to null. Calls made directly through the base class still bypass that check. Removing the escape hatch entirely would require replacing the existing inheritance design rather than extending it.

Compatibility

Existing code can continue to use wil::zstring_view and wil::zwstring_view with the same source syntax and nullable default behavior. Adding the defaulted Traits parameter changes the compiler-generated linker name for functions that expose basic_zstring_view in a binary interface. Default construction also now runs the policy-selection constructor instead of being a trivial operation. Object layout, size, and trivial copyability remain unchanged.

Tests

The focused tests cover both char and wchar_t variants:

  • nullable and non-null default construction;
  • deleted and fail-fast null construction paths;
  • implicit and explicit cross-variant conversions;
  • mutable std::basic_string_view base-reference compatibility;
  • preservation through substr(pos);
  • custom underlying character traits;
  • str_raw_ptr and std::format integration;
  • size and trivial-copyability checks.

Local validation:

  • MSVC Debug C++17 witest.exe "[zstring_view]": 155 assertions passed.
  • MSVC Debug C++23 witest.cpplatest.exe "[zstring_view]": 159 assertions passed.
  • Both builds completed without compiler warnings.

Scope

This PR does not add new string literals, change SAL annotations, alter the existing nullable aliases, or redesign basic_zstring_view to remove public inheritance.

The API direction originated in the compatibility and traits discussion on #635; this description is intended to stand on its own.

Monroe Thomas and others added 5 commits August 19, 2026 13:45
Add a traits policy that preserves the underlying char_traits type while enforcing non-null construction. Provide narrow and wide aliases, checked cross-variant conversion, and focused invariant, reference-conversion, custom-traits, formatting, and fail-fast tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229
Detect a null pointer when c_str() is called after mutation through the public string_view base, and cover the inheritance escape hatch with a regression test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229
Route nonnull pointer checks through FAIL_FAST_IF_NULL so diagnostics retain the checked expression and static analysis receives the pointer-specific contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229
Gate cross-policy conversions on the exact string_view base type, reject incompatible specializations, and limit rebinding to explicitly marked policy traits. Use a debug assertion rather than a partial production fail-fast for base-class mutation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229

@dunhor Duncan Horn (dunhor) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm liking the way that this looks. My primary concern is around the non-intuitive complexity that I'm pretty sure exists for the tests. I offer a suggestion that I believe should both work with the tests and simplify the code. The other comments are more minor.

Comment thread include/wil/stl.h
Comment on lines +174 to +175
@note basic_zstring_view publicly inherits from std::basic_string_view. A caller can explicitly cast to a mutable
base reference and assign a view with null data, bypassing the policy. Avoid mutating the object through a base

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is more general than this and is not specific to the nonnull type. E.g. you can assign non-null terminated data to both zstring_view and nonnull_zstring_view in this manner

Comment thread include/wil/stl.h
static constexpr bool empty_strings_are_non_null = true;
};

namespace details

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing /// @cond and /// @endcond pair

Comment thread include/wil/stl.h
};

template <typename TChar, typename Traits>
struct zstring_view_traits<TChar, Traits, std::void_t<decltype(Traits::empty_strings_are_non_null)>>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct zstring_view_traits<TChar, Traits, std::void_t<decltype(Traits::empty_strings_are_non_null)>>
struct zstring_view_traits<TChar, Traits, std::void_t<decltype(Traits::empty_strings_are_non_null), typename Traits::char_traits>>

Otherwise this would fail if not provided.

Comment thread include/wil/stl.h
Comment on lines +287 to +288
template <typename T = Traits, std::enable_if_t<details::zstring_view_traits<TChar, T>::empty_strings_are_non_null, int> = 0>
basic_zstring_view(std::nullptr_t) = delete;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SFINAE is probably unnecessary here if I'm understanding things correctly. For zstring_view, construction with nullptr will forward to the TChar* constructor, which is UB for null pointers, so any existing callers are guaranteed to be wrong. It's worth noting that the nullptr_t constructor is deleted starting in C++23 as well. My vote is to unconditionally delete this and keep default construction as the only (reasonable) way to get a null pointer.

Comment thread include/wil/stl.h
std::enable_if_t<
!std::is_same_v<Traits, OtherTraits> && std::is_same_v<BaseType, typename basic_zstring_view<TChar, OtherTraits>::BaseType> &&
(!ZStringViewTraits::empty_strings_are_non_null || details::zstring_view_traits<TChar, OtherTraits>::empty_strings_are_non_null),
int> = 0>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why so many different types used with enable_if? Should just be consistent with what was there before with * = nullptr

Comment thread include/wil/stl.h
Comment on lines +355 to +358
if constexpr (ZStringViewTraits::empty_strings_are_non_null)
{
WI_ASSERT(this->data() != nullptr);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if constexpr (ZStringViewTraits::empty_strings_are_non_null)
{
WI_ASSERT(this->data() != nullptr);
}
WI_ASSERT(!ZStringViewTraits::empty_strings_are_non_null || (this->data() != nullptr));

Unless this triggers a bunch of "conditional expression is constant" warnings, I'd say to optimize for lines of code for debug-only statements.

Comment thread include/wil/stl.h
!std::is_same_v<Traits, OtherTraits> && std::is_same_v<BaseType, typename basic_zstring_view<TChar, OtherTraits>::BaseType> &&
(!ZStringViewTraits::empty_strings_are_non_null || details::zstring_view_traits<TChar, OtherTraits>::empty_strings_are_non_null),
int> = 0>
constexpr basic_zstring_view(const basic_zstring_view<TChar, OtherTraits>& other) noexcept :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've added these "converting constructors" but did not do the same for the assignment operator. Consider if that should also be covered.

Comment thread include/wil/stl.h
if (value == nullptr)
{
return &details::zstring_view_empty_storage<TChar>[0];
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain I know what this is trying to do and why, however someone less familiar with how the tests are structured and work could easily look at this and think there's a mistake or something of that nature, so I'd like to try and reduce the complexity here, which I believe should be possible. The best suggestion I have at the moment is to change this to something more like:

template <bool CheckTerminator>
void check()
{
    [[maybe_unused]] auto ptr = this->data();
    [[maybe_unused]] auto len = this->size();
    if constexpr(CheckTerminator && ZStringViewTraits::empty_strings_are_non_null)
    {
        WI_STL_FAIL_FAST_IF(!ptr || (ptr[len] != 0));
    }
    else if constexpr (ZStringViewTraits::empty_strings_are_non_null)
    {
        WI_STL_FAIL_FAST_IF(!ptr);
    }
    else if constexpr (CheckTerminator)
    {
        WI_STL_FAIL_FAST_IF(ptr[len] != 0);
    }
}

Effectively, this combines the two checks - null and null terminated - into a single fail-fast check. That is, you wouldn't have the issue where a "fail-fast" would get issued, recorded in the test, and then continue execution only to crash on a null pointer read. You could then modify the constructors as follows (require_non_null is assumed to no longer exist):

  • Default constructor: no change needed
  • Copy constructor/assignment operator: no change needed
  • Pointer+length constructor: call check<true>() in the body
  • Array constructor: no change needed
  • nullptr_t constructor: delete unconditionally; see the other comment
  • Convertible to const TChar* constructor: call check<false>() in the body
  • basic_string constructor: no change needed
  • "String-like" (has c_str and size) constructor: call check<false>() in the body
  • "Path-like" (has c_str but no size) constructor: call check<false>() in the body
  • Non-explicit conversion constructor: no change needed
  • explicit conversion constructor: call check<false>() in the body
  • Deleted conversion constructor: no change needed

Comment thread include/wil/stl.h
Comment on lines +267 to 278
if constexpr (ZStringViewTraits::empty_strings_are_non_null)
{
// The test harness records fail-fast and returns, so do not dereference a rejected null pointer afterward.
if ((pStringData != nullptr) && (pStringData[stringLength] != 0))
{
WI_STL_FAIL_FAST_IF(true);
}
}
else if (pStringData[stringLength] != 0)
{
WI_STL_FAIL_FAST_IF(true);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you take my suggestion from down below, this all simplifies to a single call to check<true>()

Comment thread include/wil/stl.h
Comment on lines +30 to +32
#ifndef WI_STL_FAIL_FAST_IF_NULL
#define WI_STL_FAIL_FAST_IF_NULL FAIL_FAST_IF_NULL
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the other definition was to work around a conflict with FAIL_FAST_IF. AFAIK such a conflict doesn't exist for FAIL_FAST_IF_NULL. That said, if you take my suggestion, this define isn't needed anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants