search

microsoft / cppwinrt

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

Add support for registering event handlers with shared_ptr and weak_ptr #1330

Closed sylveon closed 1 year ago

sylveon commented 1 year ago

Fixes #1316

kennykerr commented 1 year ago

The implementation seems reasonable, but I'll let @dmachaj sign off.

dmachaj commented 1 year ago

Do either of you know if these delegates are pruned by the linker if there is no usage? The only meta-question I have is regarding binary size. If these delegates have two additional vtable slots each, and they are not pruned by the linker when unused, then that might add up across a large code base.

dmachaj commented 1 year ago

@sylveon thank you for taking this on 🙂. I am now back from my vacation and it was a nice thing to see this PR at the top of my inbox. I think your changes here (especially the test coverage) are better than anything I would have been able to write 👍.

sylveon commented 1 year ago

Since all the implementation is templated constructors, I expect them to never even show up to the linker until used (because the compiler wouldn't instantiate the template for no reason).

And you're welcome @dmachaj! I had some free time this weekend so I decided why not do this :)

kennykerr commented 1 year ago

If these delegates have two additional vtable slots each

This does not affect binary (vtable) layout.