Skip to content
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.

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

std::format support for hstring, IStringable and more #1018

Closed
JaiganeshKumaran opened this issue Sep 14, 2021 · 12 comments · Fixed by #1025
Closed

std::format support for hstring, IStringable and more #1018

JaiganeshKumaran opened this issue Sep 14, 2021 · 12 comments · Fixed by #1025

Comments

@JaiganeshKumaran
Copy link
Contributor

C++ 20 introduces std::format however currently Windows Runtime types don't work directly with them. Consider providing support for directly passing hstring and IStringable to std::format and additionally I would like to see hstring work with std::format_to so you can avoid an extra copy from std::wstring.

@MikeGitb
Copy link

On a related note: Shouldn't types like h string be implicitly convertible to a std::string_view (wither wstring_view or u16string_view). An implicit conversion to wstring_view would also enable compatibility with std::format.

@sylveon
Copy link
Contributor

sylveon commented Sep 14, 2021

hstring already has a conversion to wstring_view

@MikeGitb
Copy link

Sorry, overlooked it in the documentation. Then why doesn't it (supposedly) work with std::format?

@sylveon
Copy link
Contributor

sylveon commented Sep 14, 2021

I believe std::format ignores implicit conversions when looking for a std::formatter specialization.

@MikeGitb
Copy link

No idea, what the spec says, but this seems to work: https://godbolt.org/z/3zrWKEYc9

@sylveon
Copy link
Contributor

sylveon commented Sep 14, 2021

This does not: https://godbolt.org/z/M868GKv74

@MikeGitb
Copy link

Stupid me. The first parameter is the format string and has nothing to do with std::formatter. Sorry for the noise

@sylveon
Copy link
Contributor

sylveon commented Sep 15, 2021

Looking at it now, support for IStringable would require some level of codegen assistance. As mentionned before std::formatter selection only works with the concrete type, it doesn't consider implicit conversions or the inheritance hierarchy. There would need to be a specialization of std::formatter generated by cppwinrt for each runtime class which declares that it implements IStringable in metadata. Like std::hash specializations, this could probably be disabled by WINRT_LEAN_AND_MEAN.

Support for hstring is rather trivial, on the other hand.

additionally I would like to see hstring work with std::format_to so you can avoid an extra copy from std::wstring.

This is impossible as HSTRING is immutable. However, cppwinrt has winrt::impl::hstring_builder which allows to build a HSTRING without copies. This type could be moved out of the impl namespace to be used std::format_to. Would need a few changes to make it less error prone (right now it works, but it's easy to create a buffer overflow with it).

@kennykerr
Copy link
Collaborator

As you noted about std::hash, this requires code gen. WINRT_LEAN_AND_MEAN was added because things like std::hash were seldom used but ended up being very costly at compile time. This looks like it would be similarly expensive, so I would be reluctant to add it.

@sylveon
Copy link
Contributor

sylveon commented Sep 17, 2021

I actually have something working now, I was going to send a PR this afternoon

@sylveon
Copy link
Contributor

sylveon commented Feb 6, 2022

Leaving this here for anybody that might be interested: you can directly format into an hstring with code like this

template<std::size_t N, typename CharT, typename Traits = std::char_traits<CharT>>
struct basic_fixed_string
{
	consteval basic_fixed_string(const CharT (&arr)[N + 1]) noexcept
	{
		for (std::size_t i = 0; i < N; ++i)
		{
			data[i] = arr[i];
		}
	}

	constexpr operator std::basic_string_view<CharT, Traits>() const noexcept
	{
		return { data, N };
	}

	CharT data[N]{};
};

template<std::size_t N, typename CharT>
basic_fixed_string(const CharT (&)[N]) -> basic_fixed_string<N - 1, CharT, std::char_traits<CharT>>;

template<std::size_t N>
using fixed_string = basic_fixed_string<N, char>;

#ifdef __cpp_lib_char8_t
template<std::size_t N>
using fixed_u8string = basic_fixed_string<N, char8_t>;
#endif

template<std::size_t N>
using fixed_u16string = basic_fixed_string<N, char16_t>;

template<std::size_t N>
using fixed_u32string = basic_fixed_string<N, char32_t>;

template<std::size_t N>
using fixed_wstring = basic_fixed_string<N, wchar_t>;

template<fixed_wstring FormatString, typename... Args>
winrt::hstring hstring_format(Args&&... args)
{
	const auto sizetSize = std::formatted_size(FormatString, args...);
	uint32_t u32Size = 0;
	winrt::check_hresult(SizeTToUInt32(sizetSize, &u32Size));

	// requires wil. can be replaced by your RAII wrapper of preference calling WindowsDeleteStringBuffer
	wil::unique_hstring_buffer bufHandle;
	wchar_t* buf = nullptr;
	winrt::check_hresult(WindowsPreallocateStringBuffer(u32Size, &buf, bufHandle.put()));

	const auto result = std::format_to_n(buf, u32Size, FormatString, args...);
	assert(result.size == u32Size);

	winrt::hstring str;
	winrt::check_hresult(WindowsPromoteStringBuffer(bufHandle.get(), reinterpret_cast<HSTRING*>(winrt::put_abi(str))));
	bufHandle.release(); // we shouldn't free the HSTRING_BUFFER if WindowsPromoteStringBuffer succeeded.
	return str;
}

const auto str = hstring_format<L"{}">(1); // L"1"

The usage of fixed_string is because formatted_size and format_to_n require a compile-time string since P2216, and there are no v-prefixed alternatives that support runtime strings (IIRC that's a DR, but even if that's fixed we would lose out on compile-time format string checks). std::format_string could have allowed us to keep the same API, but got turned into an exposition only-type before being added to the standard. fixed_string passes the string as a NTTP, keeping it compile-time.

If std::format_string gets turned into a public type instead of exposition only in the future, we could provide winrt::[v]format, sharing the same exact API and compile-time checks as std::format.

@sylveon
Copy link
Contributor

sylveon commented Oct 1, 2022

This could be revisited once microsoft/STL#2937 lands in VS. The idea would be similar to:

template <typename... Args>
hstring format(std::wformat_string fmt, Args&&... args)
{
	const auto sizetSize = std::formatted_size(FormatString, args...);
	uint32_t u32Size = 0;
	winrt::check_hresult(SizeTToUInt32(sizetSize, &u32Size)); // TODO: avoid using this Win32 API

	impl::hstring_builder output(u32Size);

	const auto result = std::format_to_n(buf, u32Size, FormatString, args...);
	WINRT_ASSERT(result.size == u32Size);

	return output.to_hstring();
}

So winrt::format's API would be 1:1 with std::format, except that it returns an hstring instead.

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 a pull request may close this issue.

4 participants