unbox_value_or: Replace param::hstring with hstring

[justanotheranonymoususer]

Partially fixes: #1527
Similar to #1530
@DefaultRyan please review

[sylveon]

This is a performance regression, the following code which would previously use a fast-pass string (and be allocation-free) will now allocate a string on the heap:

auto foo = unbox_value_or<hstring>(nullptr, L"bar");

[justanotheranonymoususer]

I don't think this is actually a regression. Let's compare the old and new code for unbox_value_or<hstring>(nullptr, L"bar"):

Old code (param::hstring):

1. L"bar" > param::hstring: WindowsCreateStringReference creates a fast-pass HSTRING on the stack
2. nullptr fails the check, falls through to return *(hstring*)(&default_value)
3. Return-by-value invokes hstring copy constructor > WindowsDuplicateString on the fast-pass handle > heap allocation (fast-pass strings can't be refcounted, so duplication always copies)

New code (D&&):

1. L"bar" stays as const wchar_t(&)[4] --- no conversion
2. nullptr fails the check, falls through to return hstring(std::forward<D>(default_value))
3. hstring(L"bar") > WindowsCreateString > heap allocation

Both paths heap-allocate when the default is used. The function returns hstring by value, so there's no way to hand a fast-pass string to the caller (the stack-backed HSTRING_HEADER would be destroyed on return).

The new code is actually better on the success path: zero cost vs. a WindowsCreateStringReference + WindowsDeleteString round-trip that gets thrown away.

[sylveon]

Return-by-value invokes hstring copy constructor

hstring has a move constructor that avoids calling WindowsDuplicateString, which would be used here, no?

[justanotheranonymoususer]

*(hstring*)(&default_value) is an lvalue (a dereference expression). Implicit move on return only applies to id-expressions naming a local variable or parameter. So the copy constructor is called, not the move constructor.

return *(hstring*)(&default_value);  // lvalue > copy ctor > WindowsDuplicateString

If it were a named local, implicit move would kick in:

hstring result = ...;
return result;  // id-expression naming local > move ctor (no WindowsDuplicateString)

So the old code does copy-construct through WindowsDuplicateString, which promotes the fast-pass string to a heap-allocated one. Both old and new code allocate on the failure path.

Let me know if I'm missing something. Also intuitively, I can't see how const wchar_t(&)[4] > param::hstring > hstring can happen without allocation.

[sylveon]

Hmmm yeah you're right, can't move out of a lvalue ref.

[justanotheranonymoususer]

@DefaultRyan I believe it's ready. Anything else I should do?

[justanotheranonymoususer]

Ping, what do I need to do to have it reviewed and merged?

[Copilot]

Pull request overview

This PR updates the winrt::unbox_value_or overload for winrt::hstring so the fallback/default value is taken as a type constructible into hstring (instead of param::hstring), avoiding aborts when callers pass non-null-terminated std::wstring_view fallbacks (related to #1527).

Changes:

• Generalize unbox_value_or’s hstring fallback parameter from param::hstring const& to a forwarding reference constrained by std::is_constructible_v<hstring, D>.
• Change the fallback return path to construct an owning hstring from the provided default value.
• Add tests covering unbox_value_or<hstring> with multiple string fallback types, including a non-null-terminated std::wstring_view.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
test/test/box_string.cpp	Adds regression/behavior tests for unbox_value_or<hstring> across fallback string input types, including non-null-terminated wstring_view.
strings/base_reference_produce.h	Updates unbox_value_or’s hstring overload to accept any D constructible into hstring and construct the fallback safely.

[justanotheranonymoususer]

@StephanTLavavej do you have tips on getting it merged? Your #1543 contribution helped, at least it's not closed, but it's a small follow-up change (after #1530) I can't get to get merged :(