Skip to content

Enable using string views with wil::str_concat #590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonwis merged 11 commits into from
Jan 14, 2026
Merged

Enable using string views with wil::str_concat #590

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c0670e8
Enable combine/append of string view things
jonwis May 25, 2025
85a381a
Add a case
jonwis Dec 30, 2025
75727f9
Merge remote-tracking branch 'origin/master' into user/jonwis/string-…
jonwis Dec 30, 2025
704ffe4
Merge branch 'master' into user/jonwis/string-append-views
dunhor Jan 8, 2026
a8c87de
Merge branch 'master' into user/jonwis/string-append-views
dunhor Jan 12, 2026
ec3c068
PRFB - Move into "details" namespace
jonwis Jan 13, 2026
5dc5e5f
PRFB 2 - Move the details under the right if
jonwis Jan 13, 2026
18ec4f9
Merge branch 'master' into user/jonwis/string-append-views
dunhor Jan 13, 2026
a80eb35
Merge branch 'master' into user/jonwis/string-append-views
jonwis Jan 13, 2026
2748fe4
Appease the formatting observers
jonwis Jan 13, 2026
45fb091
Merge branch 'master' into user/jonwis/string-append-views
dunhor Jan 13, 2026
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
29 changes: 15 additions & 14 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"installDir": "${sourceDir}/build/install/${presetName}",
"toolchainFile": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake",
"cacheVariables": {
"WIL_ENABLE_ASAN": true,
"CMAKE_CONFIGURATION_TYPES": "Debug;RelWithDebInfo;Release;MinSizeRel",
"CMAKE_CXX_COMPILER": "cl",
"CMAKE_C_COMPILER": "cl"
Expand All @@ -29,29 +30,33 @@
}
},
"cacheVariables": {
"WIL_ENABLE_ASAN": false,
"CMAKE_CXX_COMPILER": "clang-cl",
"CMAKE_C_COMPILER": "clang-cl"
}
},
{
"name": "clang-release",
"inherits": "clang",
"hidden": false,
"cacheVariables": {
"WIL_ENABLE_ASAN": true,
"WIL_ENABLE_UBSAN": true
}
}
],
"buildPresets": [
{
"name": "msvc-debug",
"displayName": "MSVC Debug",
"configurePreset": "msvc",
"configuration": "Debug",
"cacheVariables": {
"WIL_ENABLE_ASAN": true
}
"configuration": "Debug"
},
{
"name": "msvc-release",
"displayName": "MSVC Release (debuggable)",
"configurePreset": "msvc",
"configuration": "RelWithDebInfo",
"cacheVariables": {
"WIL_ENABLE_ASAN": true
}
"configuration": "RelWithDebInfo"
},
{
"name": "clang-debug",
Expand All @@ -62,12 +67,8 @@
{
"name": "clang-release",
"displayName": "clang Release (debuggable)",
"configurePreset": "clang",
"configuration": "RelWithDebInfo",
"cacheVariables": {
"WIL_ENABLE_ASAN": true,
"WIL_ENABLE_UBSAN": true
}
"configurePreset": "clang-release",
"configuration": "RelWithDebInfo"
}
],
"testPresets": [
Expand Down
51 changes: 43 additions & 8 deletions include/wil/resource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1836,6 +1836,28 @@ PCWSTR str_raw_ptr(const unique_any_t<T>& val)
return str_raw_ptr(val.get());
}

struct string_view_t
Comment thread
jonwis marked this conversation as resolved.
Outdated
{
wchar_t const* data;
size_t length;
};

inline string_view_t view_from_string(string_view_t const& s)
{
return s;
}

inline string_view_t view_from_string(PCWSTR str)
{
return string_view_t{str, str ? wcslen(str) : 0};
}

template <typename T>
inline string_view_t view_from_string(wil::unique_any_t<T> const& str)
{
return view_from_string(str.get());
}

#if !defined(__WIL_MIN_KERNEL) && !defined(WIL_KERNEL_MODE)
/// @cond
namespace details
Expand All @@ -1847,12 +1869,12 @@ namespace details
// Concatenate any number of strings together and store it in an automatically allocated string. If a string is present
// in the input buffer, it is overwritten.
template <typename string_type>
HRESULT str_build_nothrow(string_type& result, _In_reads_(strCount) PCWSTR* strList, size_t strCount)
HRESULT str_build_nothrow_(string_type& result, _In_reads_(strCount) string_view_t const* strList, size_t strCount)
{
size_t lengthRequiredWithoutNull{};
for (auto& string : make_range(strList, strCount))
{
lengthRequiredWithoutNull += string ? wcslen(string) : 0;
lengthRequiredWithoutNull += string.length;
}

details::string_maker<string_type> maker;
Expand All @@ -1862,22 +1884,23 @@ namespace details
auto bufferEnd = buffer + lengthRequiredWithoutNull + 1;
for (auto& string : make_range(strList, strCount))
{
if (string)
if (string.data)
{
RETURN_IF_FAILED(StringCchCopyExW(buffer, (bufferEnd - buffer), string, &buffer, nullptr, STRSAFE_IGNORE_NULLS));
RETURN_IF_FAILED(StringCchCopyNExW(
buffer, (bufferEnd - buffer), string.data, string.length, &buffer, nullptr, STRSAFE_IGNORE_NULLS));
}
}

result = maker.release();
return S_OK;
}

// NOTE: 'Strings' must all be PCWSTR, or convertible to PCWSTR, but C++ doesn't allow us to express that cleanly
// NOTE: 'Strings' must all be string_view_t, or convertible to string_view_t, but C++ doesn't allow us to express that cleanly
Comment thread
jonwis marked this conversation as resolved.
Outdated
template <typename string_type, typename... Strings>
HRESULT str_build_nothrow(string_type& result, Strings... strings)
{
PCWSTR localStrings[] = {strings...};
return str_build_nothrow(result, localStrings, sizeof...(Strings));
string_view_t localStrings[] = {strings...};
return str_build_nothrow_<string_type>(result, localStrings, ARRAYSIZE(localStrings));
}
} // namespace details
/// @endcond
Expand All @@ -1888,7 +1911,7 @@ template <typename string_type, typename... strings>
HRESULT str_concat_nothrow(string_type& buffer, const strings&... str)
{
static_assert(sizeof...(str) > 0, "attempting to concatenate no strings");
return details::str_build_nothrow(buffer, details::string_maker<string_type>::get(buffer), str_raw_ptr(str)...);
return details::str_build_nothrow(buffer, view_from_string(details::string_maker<string_type>::get(buffer)), view_from_string(str)...);
}
#endif // !defined(__WIL_MIN_KERNEL) && !defined(WIL_KERNEL_MODE)

Expand Down Expand Up @@ -4929,6 +4952,18 @@ inline PCWSTR str_raw_ptr(const unique_hstring& str)
return str_raw_ptr(str.get());
}

inline string_view_t view_from_string(HSTRING str)
{
UINT32 length = 0;
PCWSTR rawBuffer = WindowsGetStringRawBuffer(str, &length);
return string_view_t{rawBuffer, length};
}

inline string_view_t view_from_string(const unique_hstring& str)
{
return view_from_string(str.get());
}

#endif // __WIL__WINSTRING_H_
#if (defined(__WIL__WINSTRING_H_) && !defined(__WIL__WINSTRING_H_STL) && defined(WIL_RESOURCE_STL)) || defined(WIL_DOXYGEN)
/// @cond
Expand Down
23 changes: 23 additions & 0 deletions include/wil/stl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ inline PCWSTR str_raw_ptr(const std::wstring& str)
return str.c_str();
}

inline string_view_t view_from_string(std::wstring_view const& s)
Comment thread
jonwis marked this conversation as resolved.
Outdated
{
return string_view_t{s.data(), s.length()};
}

inline string_view_t view_from_string(std::wstring const& s)
{
return string_view_t{s.data(), s.length()};
}

#if __cpp_lib_string_view >= 201606L
/**
zstring_view. A zstring_view is identical to a std::string_view except it is always nul-terminated (unless empty).
Expand Down Expand Up @@ -241,6 +251,19 @@ class basic_zstring_view : public std::basic_string_view<TChar>
using zstring_view = basic_zstring_view<char>;
using zwstring_view = basic_zstring_view<wchar_t>;

// str_raw_ptr is an overloaded function that retrieves a const pointer to the first character in a string's buffer.
// This is the overload for std::wstring. Other overloads available in resource.h.
template <typename TChar>
inline auto str_raw_ptr(basic_zstring_view<TChar> str)
{
return str.c_str();
}

inline string_view_t view_from_string(zwstring_view const& s)
{
return string_view_t{s.data(), s.length()};
}

inline namespace literals
{
constexpr zstring_view operator""_zv(const char* str, std::size_t len) noexcept
Expand Down
23 changes: 23 additions & 0 deletions tests/FileSystemTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,21 @@
// TODO: str_raw_ptr is not two-phase name lookup clean (https://github.com/Microsoft/wil/issues/8)

@dunhor dunhor Jan 8, 2026 •
edited
Loading

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.

It's been too long since I've last thought about this issue, but I wonder - if we're introducing a new function, is there's something we can do to fix this for view_from_string. Reading my past comments in #8, it looks like there's a dependency in existing code on implicit conversions, namely T -> PCWSTR and T -> std::wstring, which makes things tricky... I'll have to noodle on this a bit more, but I'm curious if we can switch to a class template1 and have the base implementation assume that T is convertible to PCWSTR. This leaves out types that are implicitly convertible to std::wstring, but hopefully there are few enough instances that we can just fix them with static_cast (or more ideally, function calls). My comments in #8 seem to suggest that the main reliance on this conversion was with std::filesystem::path, in which case explicitly calling .native() is probably more ideal anyway since it returns by reference. That said, this is an experiment we'd probably have to carry out in the OS repo to gain better confidence.

[1] Function templates don't allow partial specialization and are therefore not suitable here.

@dunhor dunhor Jan 8, 2026

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.

Side note: we can also maybe provide a specialization for std::filesystem::path in stl.h. The issue there is that due to annoying reasons there's no knowing which <filesystem> we'll be picking up. My hope here is that maybe feature test macros could be used, but I haven't dug into it.

Another possibility is to have detection logic in the base class template for something that is "string view like." E.g. something that has .data() and .size() members, though we need to be careful there - std::array has both .data() and .size() members, so we'd want other checks, for example checking for .length() (perhaps instead of .size()), though I'm not sure if that matches other types we don't want to match against.

@dunhor dunhor Jan 8, 2026

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.

Although it seems my memory is wrong... std::filesystem::path doesn't have .data()/.size() like I thought... It does have .c_str() - maybe we could detect that, although that's wasteful since we know the length... Otherwise, it does have .native() but that's too generic of a name to make assumptions about...

namespace wil
{
struct string_view_t;

PCWSTR str_raw_ptr(HSTRING);
string_view_t view_from_string(HSTRING);
#ifdef WIL_ENABLE_EXCEPTIONS
PCWSTR str_raw_ptr(const std::wstring&);
string_view_t view_from_string(std::wstring const&);
string_view_t view_from_string(std::wstring_view const&);
#endif
} // namespace wil

#ifdef WIL_ENABLE_EXCEPTIONS
#include <wil/stl.h>
#endif

#include <wil/resource.h>
#include <wil/filesystem.h>

Expand Down Expand Up @@ -528,6 +537,20 @@ TEST_CASE("FileSystemTests::VerifyStrConcat", "[filesystem]")
REQUIRE_SUCCEEDED(wil::str_concat_nothrow(combinedStringNT, test2.c_str(), test3));
REQUIRE(CompareStringOrdinal(combinedStringNT.get(), -1, expectedStr, -1, TRUE) == CSTR_EQUAL);
}

#ifdef WIL_ENABLE_EXCEPTIONS
SECTION("Concat with views of things")
{
auto part1 = std::wstring_view(L"Test2");
auto part2 = std::wstring(L"Test3");
auto part3 = wil::zwstring_view(L"Test4");
auto part4 = wil::make_unique_string_nothrow<wil::unique_cotaskmem_string>(L"Test5");
auto part5 = wil::make_unique_string_nothrow<wil::unique_hstring>(L"Test6");
wil::unique_cotaskmem_string combinedStringNT = wil::make_unique_string_nothrow<wil::unique_cotaskmem_string>(L"Test1");
REQUIRE_SUCCEEDED(wil::str_concat_nothrow(combinedStringNT, part1, part2, part3, part4, part5));
REQUIRE(CompareStringOrdinal(combinedStringNT.get(), -1, L"Test1Test2Test3Test4Test5Test6", -1, TRUE) == CSTR_EQUAL);
}
#endif
}

TEST_CASE("FileSystemTests::VerifyStrPrintf", "[filesystem]")
Expand Down