From 19ef211fc2d960e8a501627935c01170a4227712 Mon Sep 17 00:00:00 2001 From: Raymond Chen Date: Fri, 19 Mar 2021 17:49:49 -0700 Subject: [PATCH 1/4] Refactor capture into capture_to to avoid repetition The logic for deciding how to capture things is now moved into capture_to, so that we don't have to repeat it four times: (Free function vs member function) x (capture vs try_capture). --- strings/base_com_ptr.h | 77 +++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/strings/base_com_ptr.h b/strings/base_com_ptr.h index d8b86d2c8..674f36ce6 100644 --- a/strings/base_com_ptr.h +++ b/strings/base_com_ptr.h @@ -1,4 +1,22 @@ +WINRT_EXPORT namespace winrt +{ + template + struct com_ptr; +} + +namespace winrt::impl +{ + template + int32_t capture_to(void**result, F function, Args&& ...args) + { + return function(args..., guid_of(), result); + } + + template + int32_t capture_to(void** result, com_ptr const& object, M method, Args&& ...args); +} + WINRT_EXPORT namespace winrt { template @@ -162,28 +180,16 @@ WINRT_EXPORT namespace winrt *other = m_ptr; } - template - bool try_capture(F function, Args&&...args) - { - return function(args..., guid_of(), put_void()) >= 0; - } - - template - bool try_capture(com_ptr const& object, M method, Args&&...args) - { - return (object.get()->*(method))(args..., guid_of(), put_void()) >= 0; - } - - template - void capture(F function, Args&&...args) + template + bool try_capture(Args&&...args) { - check_hresult(function(args..., guid_of(), put_void())); + return impl::capture_to(put_void(), std::forward(args)...) >= 0; } - template - void capture(com_ptr const& object, M method, Args&&...args) + template + void capture(Args&&...args) { - check_hresult((object.get()->*(method))(args..., guid_of(), put_void())); + check_hresult(impl::capture_to(put_void(), std::forward(args)...)); } private: @@ -225,33 +231,19 @@ WINRT_EXPORT namespace winrt type* m_ptr{}; }; - template - impl::com_ref try_capture(F function, Args&& ...args) + template + impl::com_ref try_capture(Args&& ...args) { void* result{}; - function(args..., guid_of(), &result); + impl::capture_to(&result, std::forward(args)...); return { result, take_ownership_from_abi }; } - template - impl::com_ref try_capture(com_ptr const& object, M method, Args&& ...args) - { - void* result{}; - (object.get()->*(method))(args..., guid_of(), &result); - return { result, take_ownership_from_abi }; - } - template - impl::com_ref capture(F function, Args&& ...args) - { - void* result{}; - check_hresult(function(args..., guid_of(), &result)); - return { result, take_ownership_from_abi }; - } - template - impl::com_ref capture(com_ptr const& object, M method, Args && ...args) + template + impl::com_ref capture(Args&& ...args) { void* result{}; - check_hresult((object.get()->*(method))(args..., guid_of(), &result)); + check_hresult(impl::capture_to(&result, std::forward(args)...)); return { result, take_ownership_from_abi }; } @@ -340,6 +332,15 @@ WINRT_EXPORT namespace winrt } } +namespace winrt::impl +{ + template + int32_t capture_to(void** result, com_ptr const& object, M method, Args&& ...args) + { + return (object.get()->*(method))(args..., guid_of(), result); + } +} + template void** IID_PPV_ARGS_Helper(winrt::com_ptr* ptr) noexcept { From 9507662535701328a97918dc82e40b14cfb067a8 Mon Sep 17 00:00:00 2001 From: Raymond Chen Date: Fri, 19 Mar 2021 21:02:41 -0700 Subject: [PATCH 2/4] Allow capturing from a raw pointer This saves the hassle of having to wrap the raw pointer inside a temporary com_ptr or a lambda. --- strings/base_com_ptr.h | 6 ++++++ test/old_tests/UnitTests/capture.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/strings/base_com_ptr.h b/strings/base_com_ptr.h index 674f36ce6..bcb8acc47 100644 --- a/strings/base_com_ptr.h +++ b/strings/base_com_ptr.h @@ -13,6 +13,12 @@ namespace winrt::impl return function(args..., guid_of(), result); } + template , int> = 0> + int32_t capture_to(void** result, Interface* object, M method, Args&& ...args) + { + return (object->*method)(args..., guid_of(), result); + } + template int32_t capture_to(void** result, com_ptr const& object, M method, Args&& ...args); } diff --git a/test/old_tests/UnitTests/capture.cpp b/test/old_tests/UnitTests/capture.cpp index d1c4e41de..936f5ed77 100644 --- a/test/old_tests/UnitTests/capture.cpp +++ b/test/old_tests/UnitTests/capture.cpp @@ -39,46 +39,71 @@ HRESULT __stdcall CreateCapture(int value, GUID const& iid, void** object) noexc TEST_CASE("capture") { + // Capture from global function. com_ptr a = capture(CreateCapture, 10); REQUIRE(a->GetValue() == 10); a = nullptr; a.capture(CreateCapture, 20); REQUIRE(a->GetValue() == 20); + // Capture from com_ptr + method. auto b = capture(a, &ICapture::CreateMemberCapture, 30); REQUIRE(b->GetValue() == 30); b = nullptr; b.capture(a, &ICapture::CreateMemberCapture, 40); REQUIRE(b->GetValue() == 40); + // Capture from raw pointer + method. + b = nullptr; + b = capture(a.get(), &ICapture::CreateMemberCapture, 30); + REQUIRE(b->GetValue() == 30); + b = nullptr; + b.capture(a.get(), &ICapture::CreateMemberCapture, 40); + REQUIRE(b->GetValue() == 40); + com_ptr d; REQUIRE_THROWS_AS(capture(CreateCapture, 0), hresult_no_interface); REQUIRE_THROWS_AS(d.capture(CreateCapture, 0), hresult_no_interface); REQUIRE_THROWS_AS(capture(a, &ICapture::CreateMemberCapture, 0), hresult_no_interface); REQUIRE_THROWS_AS(d.capture(a, &ICapture::CreateMemberCapture, 0), hresult_no_interface); + REQUIRE_THROWS_AS(capture(a.get(), &ICapture::CreateMemberCapture, 0), hresult_no_interface); + REQUIRE_THROWS_AS(d.capture(a.get(), &ICapture::CreateMemberCapture, 0), hresult_no_interface); } TEST_CASE("try_capture") { // Identical to the "capture" test above, just with different // error handling. + + // Capture from global function. com_ptr a = try_capture(CreateCapture, 10); REQUIRE(a->GetValue() == 10); a = nullptr; REQUIRE(a.try_capture(CreateCapture, 20)); REQUIRE(a->GetValue() == 20); + // Capture from com_ptr + method. auto b = try_capture(a, &ICapture::CreateMemberCapture, 30); REQUIRE(b->GetValue() == 30); b = nullptr; REQUIRE(b.try_capture(a, &ICapture::CreateMemberCapture, 40)); REQUIRE(b->GetValue() == 40); + // Capture from raw pointer + method. + b = nullptr; + b = try_capture(a.get(), &ICapture::CreateMemberCapture, 30); + REQUIRE(b->GetValue() == 30); + b = nullptr; + b.try_capture(a.get(), &ICapture::CreateMemberCapture, 40); + REQUIRE(b->GetValue() == 40); + com_ptr d; REQUIRE(!try_capture(CreateCapture, 0)); REQUIRE(!d.try_capture(CreateCapture, 0)); REQUIRE(!try_capture(a, &ICapture::CreateMemberCapture, 0)); REQUIRE(!d.try_capture(a, &ICapture::CreateMemberCapture, 0)); + REQUIRE(!try_capture(a.get(), &ICapture::CreateMemberCapture, 0)); + REQUIRE(!d.try_capture(a.get(), &ICapture::CreateMemberCapture, 0)); } From 6932f50299cd81acf0445611e3a92f81bf0550f8 Mon Sep 17 00:00:00 2001 From: Raymond Chen Date: Sat, 20 Mar 2021 19:46:28 -0700 Subject: [PATCH 3/4] Simpler way of detecting that O is an object pointer If O can have "pointer to members", then it's not a function pointer. --- strings/base_com_ptr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/strings/base_com_ptr.h b/strings/base_com_ptr.h index bcb8acc47..cfdacb695 100644 --- a/strings/base_com_ptr.h +++ b/strings/base_com_ptr.h @@ -13,8 +13,8 @@ namespace winrt::impl return function(args..., guid_of(), result); } - template , int> = 0> - int32_t capture_to(void** result, Interface* object, M method, Args&& ...args) + template = 0> + int32_t capture_to(void** result, O* object, M method, Args&& ...args) { return (object->*method)(args..., guid_of(), result); } From 6faffe14750df7172409540e7ac4c821561f429a Mon Sep 17 00:00:00 2001 From: Raymond Chen Date: Sun, 21 Mar 2021 07:53:07 -0700 Subject: [PATCH 4/4] MSVC doesn't like it if you use ::* on an incomplete class This forces the class to use generalized function pointers, even if it would have used optimized function pointers had it been complete. Switch to std::is_class_v and std::is_union_v to detect that a type can have member functions. --- strings/base_com_ptr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strings/base_com_ptr.h b/strings/base_com_ptr.h index cfdacb695..ed6bf0089 100644 --- a/strings/base_com_ptr.h +++ b/strings/base_com_ptr.h @@ -13,7 +13,7 @@ namespace winrt::impl return function(args..., guid_of(), result); } - template = 0> + template || std::is_union_v, int> = 0> int32_t capture_to(void** result, O* object, M method, Args&& ...args) { return (object->*method)(args..., guid_of(), result);