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

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

Merged
merged 2 commits into from
Nov 26, 2023

Conversation

oldnewthing
Copy link
Member

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

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:

```cpp
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

```cpp
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 microsoft#1371
@kennykerr
Copy link
Collaborator

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

@jonwis
Copy link
Member

jonwis commented Nov 25, 2023

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
Copy link
Collaborator

The github VMs probably got updated.

@jonwis jonwis merged commit fc587f3 into microsoft:master Nov 26, 2023
75 checks passed
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 this pull request may close these issues.

Feature request: Allow delegates consisting of weak reference + lambda
3 participants