-
Notifications
You must be signed in to change notification settings - Fork 276
feat: add make_array_of_(uuid|guid) #539
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
2e5346b
960310f
e363b7c
bd7d9a6
e080b13
b41ab25
c758246
229a503
16a2471
04511fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,126 @@ struct single_threaded_rw_property : single_threaded_property<T> | |
|
|
||
| #endif // __WIL_CPPWINRT_AUTHORING_PROPERTIES_INCLUDED | ||
|
|
||
| #if (!defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_ICLASSFACTORY) && defined(__IClassFactory_INTERFACE_DEFINED__)) || \ | ||
| defined(WIL_DOXYGEN) // class factory | ||
| /// @cond | ||
| #define __WIL_CPPWINRT_AUTHORING_INCLUDED_ICLASSFACTORY | ||
| /// @endcond | ||
|
|
||
| template <typename T, typename... Rest> | ||
| struct class_factory : winrt::implements<class_factory<T, Rest...>, IClassFactory, winrt::no_weak_ref, Rest...> | ||
| { | ||
| HRESULT __stdcall CreateInstance(IUnknown* outer, GUID const& iid, void** result) noexcept final | ||
| try | ||
| { | ||
| *result = nullptr; | ||
|
|
||
| if (!outer) | ||
| { | ||
| return winrt::make_self<T>().as(iid, result); | ||
| } | ||
| else | ||
| { | ||
| return CLASS_E_NOAGGREGATION; | ||
| } | ||
| } | ||
| CATCH_RETURN() | ||
|
|
||
| HRESULT __stdcall LockServer(BOOL lock) noexcept final | ||
| try | ||
| { | ||
| if (lock) | ||
| { | ||
| ++winrt::get_module_lock(); | ||
| } | ||
| else | ||
| { | ||
| --winrt::get_module_lock(); | ||
| } | ||
|
|
||
| return S_OK; | ||
| } | ||
| CATCH_RETURN() | ||
| }; | ||
|
|
||
| #endif // !defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_ICLASSFACTORY) && defined(__IClassFactory_INTERFACE_DEFINED__) | ||
|
|
||
| #if (!defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_COM_SERVER) && defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_ICLASSFACTORY) && defined(_COMBASEAPI_H_)) || \ | ||
| defined(WIL_DOXYGEN) // COM server | ||
| /// @cond | ||
| #define __WIL_CPPWINRT_AUTHORING_INCLUDED_COM_SERVER | ||
| /// @endcond | ||
|
|
||
| template <typename T> | ||
| WI_NODISCARD_REASON("The class is unregistered when the returned value is destructed") | ||
| unique_com_class_object_cookie | ||
| register_com_server(GUID const& guid, DWORD context = CLSCTX_LOCAL_SERVER, DWORD flags = REGCLS_MULTIPLEUSE) | ||
| { | ||
| unique_com_class_object_cookie registration; | ||
| winrt::check_hresult(CoRegisterClassObject( | ||
| guid, winrt::make<class_factory<T, winrt::no_module_lock>>().get(), context, flags, registration.put())); | ||
| return registration; | ||
| } | ||
|
|
||
| template <typename... Types> | ||
| struct clsid_array | ||
| { | ||
| std::array<GUID, sizeof...(Types)> value; | ||
| }; | ||
|
|
||
| template <typename... Ts> | ||
| WI_NODISCARD_REASON("The classes are unregistered when the returned value is destructed") | ||
| std::vector<unique_com_class_object_cookie> register_com_server( | ||
| wil::clsid_array<Ts...> const& clsids, DWORD context = CLSCTX_LOCAL_SERVER, DWORD flags = REGCLS_MULTIPLEUSE) | ||
| { | ||
| std::vector<wil::unique_com_class_object_cookie> registrations; | ||
| registrations.reserve(sizeof...(Ts)); | ||
|
|
||
| std::size_t i = 0; | ||
| (registrations.push_back(wil::register_com_server<Ts>(clsids.value[i++], context, flags | REGCLS_SUSPENDED)), ...); | ||
|
|
||
| // allow the user to keep class objects suspended if they've explicitly passed REGCLS_SUSPENDED. | ||
| if (!WI_IsFlagSet(flags, REGCLS_SUSPENDED)) | ||
| { | ||
| winrt::check_hresult(CoResumeClassObjects()); | ||
| } | ||
|
|
||
| return registrations; | ||
| } | ||
|
|
||
| namespace details | ||
| { | ||
| template <typename T> | ||
| struct has_iid | ||
| { | ||
| template <typename U = T, std::enable_if_t<std::is_same_v<GUID, std::decay_t<decltype(__uuidof(U))>>, int> = 0> | ||
| static std::true_type invoke(int); | ||
|
|
||
| template <typename U = T> | ||
| static std::false_type invoke(float); | ||
|
|
||
| static constexpr bool value = decltype(invoke(0))::value; | ||
| }; | ||
|
|
||
| template <typename T> | ||
| constexpr bool has_iid_v = has_iid<T>::value; | ||
| } | ||
|
|
||
| template <typename... Types> | ||
| constexpr auto make_array_of_uuid() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I like the names
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose that the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Seems dunhor's suggestion can answer the question? I'd expect user typing
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. guid/uuid are the most generic. clsid and iid are specializations. these are all aliases for the same thing, so using different names can be dangerous due to the false sense of type safety. |
||
| { | ||
| return clsid_array<Types...>{std::array<GUID, sizeof...(Types)>{__uuidof(Types)...}}; | ||
| } | ||
|
|
||
| template <typename... Types> | ||
| constexpr auto make_array_of_guid() | ||
| { | ||
| static_assert(!(... && details::has_iid_v<Types>), "Use make_array_of_uuid for COM class instead"); | ||
| return clsid_array<Types...>{std::array<GUID, sizeof...(Types)>{winrt::guid_of<Types>()...}}; | ||
| } | ||
|
|
||
| #endif // (!defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_COM_SERVER) && defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_ICLASSFACTORY) && defined(_COMBASEAPI_H_)) | ||
|
|
||
| #if (!defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_FOUNDATION) && defined(WINRT_Windows_Foundation_H)) || \ | ||
| defined(WIL_DOXYGEN) // WinRT / XAML helpers | ||
| /// @cond | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| //********************************************************* | ||
| // | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // This code is licensed under the MIT License. | ||
| // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF | ||
| // ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED | ||
| // TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A | ||
| // PARTICULAR PURPOSE AND NONINFRINGEMENT. | ||
| // | ||
| //********************************************************* | ||
| //! @file | ||
| //! A server lock that runs a callback once all references are released. | ||
|
|
||
| #ifndef __WIL_CPPWINRT_NOTIFIABLE_MODULE_LOCK_INCLUDED | ||
| #define __WIL_CPPWINRT_NOTIFIABLE_MODULE_LOCK_INCLUDED | ||
|
|
||
| #ifdef WINRT_BASE_H | ||
| #error You must include this header before including any winrt header | ||
| #endif | ||
|
|
||
| #ifndef WINRT_CUSTOM_MODULE_LOCK | ||
| #error You must define 'WINRT_CUSTOM_MODULE_LOCK' at the project level | ||
| #endif | ||
|
|
||
| #include <combaseapi.h> | ||
| #include <cstdint> | ||
| #include <functional> | ||
|
|
||
| namespace wil | ||
| { | ||
| struct notifiable_server_lock | ||
| { | ||
| uint32_t operator++() noexcept | ||
| { | ||
| return CoAddRefServerProcess(); | ||
| } | ||
|
|
||
| uint32_t operator--() | ||
| { | ||
| const auto ref = CoReleaseServerProcess(); | ||
| if (ref == 0 && notifier) | ||
| { | ||
| notifier(); | ||
| } | ||
|
|
||
| return ref; | ||
| } | ||
|
|
||
| template <typename Func> | ||
| void set_notifier(Func&& func) | ||
| { | ||
| notifier = std::forward<Func>(func); | ||
| } | ||
|
|
||
| void set_notifier(std::nullptr_t) noexcept | ||
| { | ||
| notifier = nullptr; | ||
| } | ||
|
|
||
| static notifiable_server_lock& instance() noexcept | ||
| { | ||
| static notifiable_server_lock lock; | ||
| return lock; | ||
| } | ||
|
|
||
| private: | ||
| notifiable_server_lock() = default; | ||
|
|
||
| std::function<void()> notifier; | ||
| }; | ||
| } // namespace wil | ||
|
|
||
| namespace winrt | ||
| { | ||
| auto& get_module_lock() noexcept | ||
| { | ||
| return wil::notifiable_server_lock::instance(); | ||
| } | ||
| } // namespace winrt | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend keeping the std::array parameter and making clsid_array a new overload.
The overload can simply pass it on to the std::array version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind that, but let's see what @dunhor vote for.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd tend to agree with @sylveon here, but I'll take it one step further - I don't really see a value for the
clsid_arraytype at all. In fact, I'd say that it harms the API overall. Consider the way that most people will use this funciton:std::array<GUID> clsids = { CLSID_Foo, CLSID_Bar }; auto unregister = wil::register_com_server<Foo, Bar>(clsids);Tying the type names of the GUIDs to the array parameter is actively harmful and violates your "don't repeat twice" principle (I'd need to provide the type names in the type of
clsid_arrayin this case).I'd say remove this type altogether. A type alias might be fine, but still probably unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benefit is when you are using
__uuidoforwinrt::guid_of, tying the types to the array allows template type deduction to work, preventing you from typing the name twice.Like this:
wil::register_com_server(wil::make_array_of_uuid<Foo, Bar>());But we definitely need to keep the untyped array overloads for examples like the one you have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, you state that you are getting ambiguous overload errors. I'm wondering if that's because the tests are calling using double braces (
{{ CLSID_A, CLSID_B, ... }}). If you switch the tests over to single braces, I wonder if that resolves the errors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The double braces are required for
std::array. We would've neededstd::initializer_listfor single braces.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::arraysupports aggregate initialization, so single braces should be fine https://godbolt.org/z/o5cPPjYKxThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, I could swear that wasn't the case in specific instances. Guess I was wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See: https://en.cppreference.com/w/cpp/language/aggregate_initialization.html#Appertainment
Specifically:
Which probably makes my previous statement about ambiguous overloads incorrect, at least as long as
clsid_arrayis an aggregate. I.e. it would need a constructor, however that constructor would not need to beexplicit(though, frankly, it's probably good to be explicit anyway)