Bug: weak_ref::get() to an implementation type doesn't compile

[sylveon]

Version

2.0.230225.1

Summary

When using a winrt::weak_ref::get() to some implementation type, a compiler error is thrown.

Reproducible example

#include <Unknwn.h>
#include <winrt/base.h>

struct __declspec(uuid("A9D68584-DB90-458E-BBA7-DB9537786276")) IFoo : IUnknown {};

struct foo : winrt::implements<foo, IFoo> {};

int main()
{
    winrt::weak_ref<foo> a;
    a.get();
}

Expected behavior

No compiler error

Actual behavior

1>C:\Projects\ConsoleApplication15\ConsoleApplication15\x64\Debug\Generated Files\winrt\base.h(6878,17): error C2440: 'static_cast': cannot convert from 'IFoo *' to 'winrt::impl::produce<T,winrt::com_ptr<IFoo>> *'
1>        with
1>        [
1>            T=foo
1>        ]
1>C:\Projects\ConsoleApplication15\ConsoleApplication15\x64\Debug\Generated Files\winrt\base.h(6878,17): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-style cast
1>C:\Projects\ConsoleApplication15\ConsoleApplication15\x64\Debug\Generated Files\winrt\base.h(4224,30): message : see reference to function template instantiation 'D *winrt::get_self<T,winrt::com_ptr<IFoo>>(const I &) noexcept' being compiled
1>        with
1>        [
1>            D=foo,
1>            T=foo,
1>            I=winrt::com_ptr<IFoo>
1>        ]
1>C:\Projects\ConsoleApplication15\ConsoleApplication15\x64\Debug\Generated Files\winrt\base.h(4211,28): message : while compiling class template member function 'winrt::com_ptr<D> winrt::weak_ref<D>::get(void) noexcept const'
1>        with
1>        [
1>            D=foo
1>        ]
1>C:\Projects\ConsoleApplication15\ConsoleApplication15\ConsoleApplication15.cpp(13,26): message : see reference to class template instantiation 'winrt::weak_ref<D>' being compiled
1>        with
1>        [
1>            D=foo
1>        ]

Additional comments

Works if I where to use a weak_ref<IFoo>, but I don't want that.

[DefaultRyan]

Note, this works when the default interface (aka first interface supplied to winrt::implements) is a WinRT interface. This only fails when the default interface is classic COM. In other words, changing your definition of foo to

struct foo : winrt::implements<foo, winrt::Windows::Foundation::IInspectable, IFoo> {};

could unblock you as a temporary workaround.

It seems this could be fixed in a low-risk fashion. The implementation of get() calls get_self<D, I>(), which assumes that I is projected and therefore has a specialization of produce<D, I>. But this isn't true when I is classic COM or a com_ptr<I>.

One fix would be to augment get_self() to handle com_ptr<I> by adding an overload:

WINRT_EXPORT namespace winrt
{
    template <typename D, typename I>
    D* get_self(com_ptr<I> const& from) noexcept
    {
        static_assert(impl::is_classic_com_interface<I>::value, "get_self can only be called with projected types or classic COM interfaces");
        return static_cast<D*>(static_cast<impl::producer<D, I>*>(from.get()));
    }
}

One could attempt to be more surgical and limit the change to weak_ref::get() by skipping get_self() if T's first interface is classic COM, but that feels slightly more hacky.

[kennykerr]

Originally, weak references were limited to IInspectable-derived interfaces and only extended to all IUnknown-derived interfaces after I convinced enough people that it would work. 🙃

But at this point, I'm not sure it's worth complicating the already very complicated implements machinery given there's a simple workaround. As it is, we already have a backlog of issues blocking updating C++/WinRT within the OS repo.

[sylveon]

From the sounds of it, the get_self overload would allow this scenario without changing the implements machinery.

[sylveon]

I just hit this issue in a different scenario, but with the same class

winrt::fire_and_forget foo::SomeCallback()
{
	const auto self_weak = get_weak();
	co_await wil::resume_foreground(m_SomeDispatcherQueue);

	if (const auto self = self_weak.get())
	{
		// do things with self
	}
}

Here the issue is not as easy to workaround, as you have no control over the kind of winrt::weak_ref that get_weak() returns.

[github-actions]

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[sylveon]

This has a PR open.

[sylveon]

This is a blocking issue for me. I cannot use the following code without the fix in the PR, or some ugly workaround (adding IInspectable gives code bloat):

struct MyClass : winrt::implements<MyClass, IClassicComInterface>
{
	winrt::fire_and_forget MyClass::OnEvent()
	{
		const auto self_weak = get_weak();
		co_await wil::resume_foreground(m_DispatcherQueue);

		if (const auto self = self_weak.get())
		{
			self->DoThing();
		}
	}
};

I cannot make a new release of my software without a fix, as I need to be able to do this.

Really disappointed about this recent change in the contributors guide.

Why is the platform that is supposed to be used for all future new C++ Windows app development (via the WASDK) left in such a pitiful state with close to zero resources?

What happened about C++/WinRT being modern C++ when backwards-compatible additions to make it keep up to date with changes to C++ are being denied (#1323)?

This brings in doubt the idea that cppwinrt will ever adopt additions such as metaclasses, which are partly being pushed sorely for projects like it (and are being promised as the fix for plenty of issues affecting cppwinrt).

What's wrong with keeping PRs open until bandwidth is available? Stale botting issues and PRs helps no one, it only achieves an illusion of low issue count while actual problems end up brushed under the rug.

Is C++/WinRT dead?

[kennykerr]

C++/WinRT is not dead. 😊 It continues to work as expected, providing a simpler way to use and implement WinRT APIs. What you're asking for here is not specific to WinRT. It's not a bad idea but it's a new feature and we simply don't have developers available to support new feature work at present. And you're not really blocked as you have a simple workaround.

[sylveon]

Of course, C++/WinRT works and I don't expect that to change. WRL and C++/CX still work too, after all.

I was thinking more about future improvements. Lots of the C++/WinRT developer experience is still suboptimal (poorly documented/often buggy IDL compiler, heavy compile times, poor IDE integration, etc.), and we have been bearing these issues for years with the promise that metaclasses/modules would fix them. If all feature work is on freeze, are we going to just have to accept these issues as is ad-eternum?

My modules PR is on hold waiting for compiler fixes (and it will fix some of the mentioned dev UX issues, by making the inner loop better and compile-time faster), but should I even bother going through with completing it and submitting it once those are fixed?

Regarding this specific problem, I don't think it's a feature. If get_weak() and implicit conversion to weak_ref works on a class that only implements classic COM interfaces (it currently does), why should weak_ref::get() be considered a separate feature and not a bug? get() not working pretty much defeats weak_ref as a whole.

None of the workarounds are something that I feel too great about shipping in production:

• Adding winrt::IInspectable as the default class leads to code bloat (I don't need the IInspectable interface). It also kills type checking (as weak_ref just resolves an IInspectable from the weak reference, then type puns it into T in that case).
• Manually declaring the proposed get_self overload means I have to tap into implementation details
• There is no way to convert weak_ref<T> to a weak_ref<U> where T : U, either

[kennykerr]

Love your passion. We need more of that at Microsoft. See: https://careers.microsoft.com/

[sylveon]

I would love to come and help! That's more of a long term thing though, especially as I am still busy with university and internships at the moment.

Are there any plans to give some resources to cppwinrt, e.g. at least one or two people that are able to work on it, or to transfer the project to a team that is able to take care of it? It feels like right now there are no resources at all for cppwinrt (which is weird, you'd think WASDK/WinUI would care as cppwinrt is the only means to consume those libraries from C++)

[kennykerr]

I've tagged a few project maintainers. If one of them is available to offer guidance and mentorship for this issue, they will reach out according to the contributing guide to discuss and agree on an approach.

@microsoft/cppwinrt-maintainers

[sylveon]

Possible to reopen #1314 as well?