Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 146 additions & 8 deletions include/wil/stl.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,70 @@ inline PCWSTR str_raw_ptr(const std::wstring& str)

#if __cpp_lib_string_view >= 201606L

// WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER (opt-in, off by default):
// When defined, basic_zstring_view{}'s data() returns a pointer to a static empty buffer
// instead of nullptr. This makes c_str() always safe to dereference and aligns with the
// design proposed for std::basic_zstring_view in P3655R0.
//
// The default-off preserves source compatibility with code that uses data() == nullptr as
// a "no string" sentinel and matches the documented default of std::basic_string_view.
// New callers that want the safer default-construct behaviour opt in by defining the macro
// ahead of including this header.
//
// Linkage matrix (enforced by WI_ODR_PRAGMA via MSVC's /detect_mismatch). The relevant
// axis is whether the macro is defined when each translation unit includes this header:
// * Defined in both: instances interop normally; default ctor returns a non-null empty buffer.
// * Undefined in both: instances interop normally; default ctor returns nullptr (base behaviour).
// * Mismatched: link error LNK2038 on the mismatch tag
// 'ODR_violation_WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER_mismatch'.
#if defined(WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER)
WI_ODR_PRAGMA("WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER", "1")
#else
WI_ODR_PRAGMA("WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER", "0")
#endif

/**
zstring_view. A zstring_view is identical to a std::string_view except it is always nul-terminated (unless empty).
* zstring_view can be used for storing string literals without "forgetting" the length or that it is nul-terminated.
* A zstring_view can be converted implicitly to a std::string_view because it is always safe to use a nul-terminated
string_view as a plain string view.
* A zstring_view can be constructed from a std::string because the data in std::string is nul-terminated.
`basic_zstring_view<TChar>` is a non-owning, read-only view of a *null-terminated* sequence of `TChar`.
The class adds null-termination guarantees to `std::basic_string_view`, making it suitable for passing
to C APIs that require dereferenceable null-terminated strings (e.g. `printf("%s", v.c_str())`,
`fopen(v.c_str(), ...)`). Where `std::basic_string_view` requires the caller to remember the
null-termination contract, `basic_zstring_view` enforces it at construction for views built from a
real buffer.

The opt-in macro `WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER` controls the default-construction behaviour.
Define it before including this header to get the stronger invariant; leave it undefined for
backward compatibility with the original WIL semantics.

With `WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER` defined:
- Every constructed view satisfies `data() != nullptr` and `data()[size()] == TChar()`.
- A default-constructed view points at an internal static empty buffer; `size() == 0` and
`c_str()[0] == TChar()`.
- `c_str()` is always safe to dereference and to hand to a C API.
- Aligns with the design proposed for `std::basic_zstring_view` in P3655R0
(https://wg21.link/p3655r0).

Without `WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER` (default):
- The invariant `data()[size()] == TChar()` applies only when `data() != nullptr`.
- A default-constructed view matches `std::basic_string_view`: `data() == nullptr`,
`size() == 0`.
- `c_str()` returns `nullptr` on a default-constructed view; callers must guard before
dereferencing.

Other behaviour (both modes):
- Decays implicitly to `std::basic_string_view` (it inherits from it).
- Constructible from a string literal, a `(const TChar*, size_type)` pair (with a debug fail-fast
verifying the null terminator), a `std::basic_string`, or any user-defined type that exposes
`c_str()` (and optionally `size()`) returning `TChar`.
- `substr(pos)` returns a `basic_zstring_view` (the tail of a null-terminated string is itself
null-terminated); follows the design proposed for `std::zstring_view` in P3655R0.
- `substr(pos, count)` returns a `std::basic_string_view` because an arbitrary slice is generally
not null-terminated.

@note Public inheritance from `std::basic_string_view` means a caller can bypass the
null-termination invariant by casting to the base, e.g.
`static_cast<std::basic_string_view<char>&>(zv) = sv` where `sv` is a non-null-terminated view.
P3655R0's proposed `std::zstring_view` avoids this by not inheriting. The invariant holds for
normal use but isn't airtight against this kind of base-class cast.
*/
template <class TChar>
class basic_zstring_view : public std::basic_string_view<TChar>
Expand Down Expand Up @@ -167,11 +225,27 @@ class basic_zstring_view : public std::basic_string_view<TChar>
};

public:
#if defined(WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER)
/**
* Default-construct a view of an internal static empty buffer.
* Yields `data() != nullptr`, `size() == 0`, and `c_str()[0] == TChar()` so the result is safe
* to hand to any C API expecting a dereferenceable null-terminated string.
*/
constexpr basic_zstring_view() noexcept : std::basic_string_view<TChar>(&_empty_storage[0], 0)
{
}
#else
/**
* Default-construct a view matching `std::basic_string_view`: `data() == nullptr`, `size() == 0`.
* Define `WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER` before including this header to flip the default to
* a non-null empty buffer (see the class documentation).
*/
constexpr basic_zstring_view() noexcept = default;
#endif
constexpr basic_zstring_view(const basic_zstring_view&) noexcept = default;
constexpr basic_zstring_view& operator=(const basic_zstring_view&) noexcept = default;

constexpr basic_zstring_view(const TChar* pStringData, size_type stringLength) noexcept :
constexpr basic_zstring_view(_In_reads_z_(stringLength + 1) const TChar* pStringData, size_type stringLength) noexcept :
std::basic_string_view<TChar>(pStringData, stringLength)
{
if (pStringData[stringLength] != 0)
Expand All @@ -189,7 +263,7 @@ class basic_zstring_view : public std::basic_string_view<TChar>
// Construct from nul-terminated char ptr. To prevent this from overshadowing array construction,
// we disable this constructor if the value is an array (including string literal).
template <typename TPtr, std::enable_if_t<std::is_convertible<TPtr, const TChar*>::value && !std::is_array<TPtr>::value>* = nullptr>
constexpr basic_zstring_view(TPtr&& pStr) noexcept : std::basic_string_view<TChar>(std::forward<TPtr>(pStr))
constexpr basic_zstring_view(_In_z_ TPtr&& pStr) noexcept : std::basic_string_view<TChar>(std::forward<TPtr>(pStr))

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.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

{
}

Expand All @@ -215,12 +289,71 @@ class basic_zstring_view : public std::basic_string_view<TChar>
return this->data()[idx];
}

WI_NODISCARD constexpr const TChar* c_str() const noexcept
WI_NODISCARD _Ret_z_ constexpr const TChar* c_str() const 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.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

{
WI_ASSERT(this->data() == nullptr || this->data()[this->size()] == 0);
return this->data();
}

// contains() backport for builds below C++23. Compiles out once the STL provides
// basic_string_view::contains natively (via __cpp_lib_string_contains).
#if !defined(__cpp_lib_string_contains) || __cpp_lib_string_contains < 202011L
Comment thread
dmachaj marked this conversation as resolved.
WI_NODISCARD constexpr bool contains(std::basic_string_view<TChar> sv) const noexcept
{
return std::basic_string_view<TChar>(*this).find(sv) != std::basic_string_view<TChar>::npos;
}

WI_NODISCARD constexpr bool contains(TChar ch) const noexcept
{
return std::basic_string_view<TChar>(*this).find(ch) != std::basic_string_view<TChar>::npos;
}

WI_NODISCARD constexpr bool contains(_In_z_ const TChar* s) const
{
return std::basic_string_view<TChar>(*this).find(s) != std::basic_string_view<TChar>::npos;
}
#endif // !defined(__cpp_lib_string_contains) || __cpp_lib_string_contains < 202011L

/**
* Returns a `basic_zstring_view` of the tail of this view, starting at `pos`.
*
* The result is null-terminated because the tail of a null-terminated string is itself
* null-terminated.
*
* @param pos starting position (default 0). Must satisfy `pos <= size()`.
* @throws std::out_of_range if `pos > size()` (propagated from `std::basic_string_view::substr`).
*/
WI_NODISCARD constexpr basic_zstring_view substr(size_type pos = 0) const
{
const auto tail = std::basic_string_view<TChar>(*this).substr(pos);
// Short-circuit the (nullptr, 0) tail case (substr(0) on a default-constructed view
// when WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER is not defined) so we don't round-trip a null
// pointer through the verifying (ptr, length) constructor.

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.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

if (tail.data() == nullptr)
{
return basic_zstring_view{};
}
return basic_zstring_view(tail.data(), tail.size());
}

// Re-declared (not just inherited) so the one-arg substr(pos) above doesn't hide the
// inherited two-arg overload via standard name-hiding rules.
/**
* Returns a `std::basic_string_view` of an arbitrary sub-range.
*
* The return type is `std::basic_string_view` rather than `basic_zstring_view` because the
* sub-range is generally not null-terminated. Callers who need a null-terminated tail should
* use the one-argument `substr(pos)` overload above.
*
* @param pos starting position. Must satisfy `pos <= size()`.
* @param count maximum number of characters in the resulting sub-range. Clamped to `size() - pos`.
* @throws std::out_of_range if `pos > size()` (propagated from `std::basic_string_view::substr`).
*/
WI_NODISCARD constexpr std::basic_string_view<TChar> substr(size_type pos, size_type count) const
{
return std::basic_string_view<TChar>(*this).substr(pos, count);
}

private:
// Bounds-checked version of char_traits::length, like strnlen. Requires that the input contains a null terminator.
static constexpr size_type length_n(_In_reads_opt_(buf_size) const TChar* str, size_type buf_size) noexcept
Expand All @@ -237,6 +370,11 @@ class basic_zstring_view : public std::basic_string_view<TChar>
// The following basic_string_view methods must not be allowed because they break the nul-termination.
using std::basic_string_view<TChar>::swap;
using std::basic_string_view<TChar>::remove_suffix;

// Backing buffer for the default-constructed view. Only present when the safer default is opted in.
#if defined(WIL_ZSV_DEFAULT_TO_EMPTY_BUFFER)
static inline constexpr TChar _empty_storage[1]{TChar()};
#endif
};

using zstring_view = basic_zstring_view<char>;
Expand Down
2 changes: 2 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,15 @@ add_subdirectory(cppwinrt-notifiable-server-lock)
add_subdirectory(noexcept)
add_subdirectory(normal)
add_subdirectory(win7)
add_subdirectory(zsvsafe)

add_test(NAME app COMMAND $<TARGET_FILE:witest.app>)
add_test(NAME cpplatest COMMAND $<TARGET_FILE:witest.cpplatest>)
add_test(NAME cppwinrt-notifiable-server-lock COMMAND $<TARGET_FILE:witest.cppwinrt-notifiable-server-lock>)
add_test(NAME noexcept COMMAND $<TARGET_FILE:witest.noexcept>)
add_test(NAME normal COMMAND $<TARGET_FILE:witest>)
add_test(NAME win7 COMMAND $<TARGET_FILE:witest.win7>)
add_test(NAME zsvsafe COMMAND $<TARGET_FILE:witest.zsvsafe>)

if (${WIL_ENABLE_ASAN})
add_subdirectory(sanitize-address)
Expand Down
Loading
Loading