microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 236 forks source link

Allow delegates to be created with weak reference + lambda #1372

Closed oldnewthing closed 9 months ago

oldnewthing commented 9 months ago

We have found that a very common pattern for event handlers is to capture a weak reference into a lambda, and in the event handler, try to upgrade the weak reference to a strong one, and if so, do some work:

widget.Closed([weak = get_weak(), data](auto&& sender, auto&& args)
{
    if (auto strongThis = weak.get())
    {
        strongThis->do_all_the_things(data);
    }
});

This commit extends the existing delegate constructors to permit a winrt::weak_ref + lambda (or std::weak_ptr + lambda), which simplifies the above to

widget.Closed({ get_weak(), [this, data](auto&& sender, auto&& args)
{
    do_all_the_things(data);
} });

Implementation notes

A lambda and pointer to member function are hard to distinguish in a template parameter list. In theory, we could use SFINAE or partial specialization, but a simpler solution is to distinguish the two inside the body of the constructor, via std::is_member_function_pointer_v.

The com_ptr and shared_ptr variants of the test were unified, since I found myself editing two nearly identical tests.

Fixes: #1371

kennykerr commented 9 months ago

Reran the test but same result. Not sure why the test is failing.

jonwis commented 9 months ago

Looking at yvals_core.h, I see this:

#if __clang_major__ < 16
_EMIT_STL_ERROR(STL1000, "Unexpected compiler version, expected Clang 16.0.0 or newer.");
#endif // ^^^ old Clang ^^^

And the build setup output says this:

Cache not found for input keys: llvm-msvc-Windows-15.0.5
Run Invoke-WebRequest "https://github.com/llvm/llvm-project/releases/download/llvmorg-15.0.5/LLVM-15.0.5-win64.exe" -OutFile LLVM-installer.exe
Run Invoke-WebRequest "https://github.com/zufuliu/llvm-utils/releases/download/v22.09/LLVM_VS20[17](https://github.com/microsoft/cppwinrt/actions/runs/6936099951/job/19011698077?pr=1372#step:3:19).zip" -OutFile LLVM_VS2017.zip

I'm new to GH actions & pipelines, but I see a reference to 15.0.5 in https://github.com/microsoft/cppwinrt/blob/master/.github/actions/setup-llvm-msvc/action.yml ... the latest LLVM hot off of winget install llvm.llvm is 17.0.5, so I guess that needs updating?

kennykerr commented 9 months ago

The github VMs probably got updated.