Skip to content

basic_zstring_view: add opt-in nonnull variants - #668

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

Monroe Thomas (mmthomas) wants to merge 8 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. Existing callers may use that null pointer 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 wil::nonnull_zstring_view and wil::nonnull_zwstring_view for that use case. Their default constructors point at an internal empty string, while default-constructed wil::zstring_view and wil::zwstring_view continue to hold null.

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

printf("%s", nonnull.c_str());

The non-null empty state also appears in the C++29 proposal P3655R5, std::cstring_view, which describes a non-owning, null-terminated string view whose default constructor refers to a static null terminator.

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 selects non-null behavior separately from the character traits, which supply operations such as character comparison and string-length calculation. Its char_traits member determines the underlying std::basic_string_view type. With the default character traits, both narrow variants derive from std::string_view, and both wide variants derive from std::wstring_view.

Custom character traits can be supplied through nonnull_zstring_view_traits<TChar, Traits>. Policy detection requires both empty_strings_are_non_null and char_traits; otherwise the supplied type is treated as ordinary character traits.

Construction, conversion, and assignment

Direct nullptr construction is deleted for both nullable and non-null variants. The nullable variants otherwise retain their original construction preconditions and failure paths. The non-null variants reject typed null pointer inputs through WIL's fail-fast mechanism, which reports the contract violation and terminates the process.

Construction Nullable variant Non-null variant
Default construction Empty, null data Empty, non-null data
Direct nullptr Compile-time error Compile-time error
Typed null pointer, without length Original invalid-input path Fail-fast
Null pointer plus zero length Original invalid-input path Fail-fast
Null pointer plus non-zero length Original invalid-input path Fail-fast
Non-null empty string buffer plus zero length Preserves the pointer and zero length Preserves the pointer and zero length
String-like object with c_str() == nullptr, size() == 0 Empty, null data Fail-fast

For non-null views, pointer-plus-length construction checks the pointer and trailing null terminator together. Initialization avoids passing a null pointer with a non-zero length to the standard-library base, so WIL can report the failure in the constructor body. Pointer-only construction similarly avoids calculating the length of a null pointer before validation.

Converting construction and assignment require matching underlying std::basic_string_view types. Conversion from a non-null view to a nullable view is implicit. The reverse conversion requires explicit construction and checks the source pointer:

wil::zstring_view source{"hello"};
wil::nonnull_zstring_view checked{source}; // explicit, checked construction
wil::zstring_view nullable = checked;      // implicit conversion

Assignment is supported in both compatible directions. Assigning a nullable view into a non-null view checks the source before modifying the destination. Same-type copy assignment stays defaulted, and construction or assignment between incompatible underlying character traits is deleted.

WIL's unit test harness intercepts fail-fast to record the failure and allow execution to continue instead of terminating the test process. When this happens during a rejected assignment, the assignment operator returns without changing the destination's pointer or size.

substr(pos) preserves the selected view type and its default-state behavior. str_raw_ptr and std::format accept the new variants as well.

Inheritance limitation

All basic_zstring_view variants publicly inherit from std::basic_string_view. A mutable base reference can therefore assign null data or a view that is not null-terminated, bypassing the derived type's validation:

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

Avoid mutating these objects through a base reference. The derived c_str() has debug assertions for the non-null policy and trailing terminator; calls made directly through the base class bypass those checks.

Compatibility

wil::zstring_view and wil::zwstring_view retain their names, nullable default state, and original runtime construction paths. Direct nullptr construction now fails to compile for both nullable and non-null variants; stronger runtime null checks are confined to the non-null policy.

Adding the defaulted Traits parameter changes the compiler-generated linker names for types and functions that expose basic_zstring_view. The object representation remains a pointer and length, and the views remain trivially copyable.

Tests

Regression coverage includes narrow and wide default states, nullable string-like empty states, non-null null-input rejection for zero and non-zero lengths, zero-length buffer validation, compile-time nullable construction and substrings, compatible and incompatible conversions and assignments, destination preservation after rejected assignment, partial-policy detection, and integration with str_raw_ptr and std::format.

The traits-policy design builds on Duncan's feedback in #635.

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 Outdated
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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in e60a42a. The warning now lives on basic_zstring_view and covers null data and loss of null termination through mutable base references for both variants.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added /// @cond and /// @endcond around the internal details block in e60a42a.

Comment thread include/wil/stl.h Outdated
};

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in e60a42a; policy detection now requires both members. Added a regression test for the missing char_traits case.

Comment thread include/wil/stl.h Outdated
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.

@mmthomas Monroe Thomas (mmthomas) Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The nullptr_t constructor is deleted unconditionally, with compile-time coverage for both variants. In 92ef222 I limited the stronger runtime null checks to the non-null policy, preserving the nullable variants' original construction paths. In particular, a string-like object returning null from c_str() and zero from size() can still construct an empty nullable view. Added regression coverage for that case.

Comment thread include/wil/stl.h Outdated
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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Standardized these overloads on std::enable_if_t<...>* = nullptr in e60a42a.

Comment thread include/wil/stl.h Outdated
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied the single assertion in e60a42a. It compiled without constant-condition warnings in the local MSVC Debug C++17 and C++23 builds.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in e60a42a. One converting assignment operator handles both directions when the underlying std::basic_string_view types match. Assigning a nullable view into a non-null view checks the source before modifying the destination.

Same-type copy assignment remains defaulted, and assignment between incompatible base types is deleted. The tests cover both compatible directions and verify that the destination's pointer and size remain unchanged if the fail-fast test hook returns.

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

@mmthomas Monroe Thomas (mmthomas) Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 92ef222. The constructors use validate_pointer() and validate_pointer_and_terminator() so the checks are apparent at each call site without a Boolean template argument. The non-null variant checks null and termination together; the nullable variant retains its original preconditions and failure paths.

For a null input to the non-null variant, I construct the std::string_view base with (nullptr, 0), then fail-fast in the constructor body. Otherwise, the base could fail before our null check runs.

Added narrow and wide regression coverage for the preserved nullable empty state, non-null rejection with zero and non-zero lengths, and compile-time nullable construction. Thanks Duncan!

Comment thread include/wil/stl.h Outdated
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>()

@mmthomas Monroe Thomas (mmthomas) Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The constructor body is now a single call to validate_pointer_and_terminator() in e60a42a.

Comment thread include/wil/stl.h Outdated
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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed WI_STL_FAIL_FAST_IF_NULL in e60a42a; the simplified validation no longer needs it.

Duncan Horn (dunhor) and others added 2 commits September 4, 2026 10:07
Validate pointer inputs in constructor bodies without substituting test-only buffers. Reject direct nullptr construction for both variants, tighten policy detection, and clarify the shared inheritance limitation.

Add compatible cross-policy assignment with pre-mutation null validation, preserving the destination when fail-fast is intercepted. Cover nullable nullptr rejection, partial policies, assignment mutation, and rejected-assignment preservation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229
@mmthomas
Monroe Thomas (mmthomas) marked this pull request as ready for review September 15, 2026 20:27
Limit runtime null validation and guarded base initialization to the nonnull policy. Preserve nullable construction paths and allow empty string-like objects to retain null data. Keep the nullable terminator guard so valid construction and substr remain constexpr-usable while reporting the actual failing expression.

Cover null and zero-length string-like inputs, nonnull null/nonzero rejection, copied and assigned nullable state, empty buffers, and compile-time narrow/wide construction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5697cd0e-cf83-4d94-9f72-4b8c79379229
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