feat: add make_array_of_(uuid|guid)

[roxk]

This is a follow up PR of #537. Requires #537 before merging.

I'm mostly fine with sylveon's improvement in #537 given the current unfortunate situation, but every time I see that we have to write class name twice during registration a little bit of me die inside:

auto revoker = wil::register_com_server<MyServer, BuggyServer>({{__uuidof(MyServer), __uuidof(BuggyServer)}});
                                             ^ 1st                            ^ 2nd

If you register N class you have to spell out the type 2N times. Not good. I consider please repeat yourself twice a sign of bad API design.

So this PR adds make_array_of_uuid and make_array_of_guid to mitigate the problem:

auto revoker = wil::register_com_server(wil::make_array_of_uuid<MyServer, BuggyServer>());

It works by wrapping the array in a template and let template parameter deduction works out the rest.

Also disallow using make_array_of_guid if the type is a COM class, i.e. __uuof(T) is defined. Shamelessly borrowed from @dunhor's has_iid_v in previous PR.

This PR does not attempt to address the issue that the implementation type might not have a default interface that is exclusive to it.

[sylveon]

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

[roxk]

I don't mind that, but let's see what @dunhor vote for.

[dunhor]

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_array type 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_array in this case).

I'd say remove this type altogether. A type alias might be fine, but still probably unnecessary.

[sylveon]

The benefit is when you are using __uuidof or winrt::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.

[dunhor]

Gotcha, I see.

[dunhor]

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.

[sylveon]

The double braces are required for std::array. We would've needed std::initializer_list for single braces.

[dunhor]

std::array supports aggregate initialization, so single braces should be fine https://godbolt.org/z/o5cPPjYKx

[sylveon]

Uh, I could swear that wasn't the case in specific instances. Guess I was wrong.

[dunhor]

See: https://en.cppreference.com/w/cpp/language/aggregate_initialization.html#Appertainment

Specifically:

Otherwise, elem is an aggregate and that subaggregate is replaced in the list of aggregate elements by the sequence of its own aggregate elements, and the appertainment analysis resumes with the first such element and the same initializer clause. In other words, these rules apply recursively to the aggregate’s subaggregates.

Which probably makes my previous statement about ambiguous overloads incorrect, at least as long as clsid_array is an aggregate. I.e. it would need a constructor, however that constructor would not need to be explicit (though, frankly, it's probably good to be explicit anyway)

[roxk]

I added wiki for register_com_server and friends. Please kindly help review it as well if maintainers have time: https://github.com/microsoft/wil/wiki/Implementing-OOP-COM-Server-with-cppwinrt

[dunhor]

I think I like the names make_clsid_array_uuid and make_clsid_array_guid (or perhaps make_clsid_array_uuidof and make_clsid_array_guid_of to make it more explicit which operation is being used to form the CLSIDs). This makes it clearer what the function's purpose is. What do others think here?

[dunhor]

I suppose that the __uuidof version could be useful outside of the scope of CLSIDs, e.g. if you were to implement something like RuntimeClass and wanted an array of IIDs when implementing QueryInterface etc.

[sylveon]

What about make_clsid_array and make_clsid_array_winrt? I don't particularly like uuid vs guid because it is not clear what are the differences.

[roxk]

What about make_clsid_array and make_clsid_array_winrt? I don't particularly like uuid vs guid because it is not clear what are the differences.

Seems dunhor's suggestion can answer the question? I'd expect user typing make_clsid_array_ and then seeing uuidof vs guid_of would be able to tell which is which

[ChrisGuzak]

guid/uuid are the most generic. clsid and iid are specializations.
it is odd that clsid_array is the building block. I'd expect guid/uuid to be at the bottom.

these are all aliases for the same thing, so using different names can be dangerous due to the false sense of type safety.

[roxk]

Note that the pattern here is "copy then move". In practice this value is always move-constructed from temporary, or in more recent standard just materialized without any temporary.

[dunhor]

std::move is pointless here. It's a std::array and GUIDs. Accept by const reference and just assign and make it constexpr while you are at it.

[dunhor]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).