microsoft / cppwinrt

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

Bug: winrt::capture needs detector for "REFIID, IUnknown**" methods #1299

Closed jonwis closed 1 year ago

jonwis commented 1 year ago

Version

No response

Summary

Trying to call https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ms718237(v=vs.85) (IGetRow::GetRowFromHROW) in my app using winrt::capture. The signature is HRESULT GetRowFromHROW(IUnknown **pUnkOuter, HROW hRow, REFIID riid, IUnknown **ppUnk).

Capture requires the capture output parameter to be void**, so this fails.

Reproducible example

#include <windows.h>
#include <propsys.h>
#include <winrt/Windows.Foundation.h>

HRESULT GetRowFromHROW(
    ::IUnknown** pUnkOuter,
    int hRow,
    REFIID riid,
    ::IUnknown** ppUnk);

int main()
{
    auto r = winrt::capture<::IPropertyStore>(GetRowFromHROW, 3, nullptr);
    r->Commit();

    return 0;
}

Expected behavior

Expected capture to adapt to three common COM capturable parameter pairs - (REFIID, void**), (REFIID, ::IUnknown**), (REFIID, IInspectable**) ... or maybe more generally "void and things-that-derive-from-IUnknown".

Actual behavior

1>c:\source\repos\App1\CppWinrtTest\Debug\Generated Files\winrt\base.h(2330,48): error C2664: 'HRESULT (IUnknown ,int,const IID &,IUnknown )': cannot convert argument 4 from 'void ' to 'IUnknown ' 1>c:\source\repos\App1\CppWinrtTest\Debug\Generated Files\winrt\base.h(2330,48): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-style cast 1>c:\source\repos\App1\CppWinrtTest\Debug\Generated Files\winrt\base.h(2567): message : see reference to function template instantiation 'int32_t winrt::impl::capture_to<IPropertyStore,HRESULT(__cdecl *)(IUnknown ,int,const IID &,IUnknown ),_Ty,nullptr>(void *,F,_Ty &&,nullptr &&)' being compiled 1> with 1> [ 1> _Ty=int, 1> F=HRESULT (__cdecl )(IUnknown ,int,const IID &,IUnknown ) 1> ]

Additional comments

Fully willing for this to be "go add this to WIL as it's a Windows SDK problem not a C++/WinRT problem."

sylveon commented 1 year ago

Seeing as this is a C++/WinRT equivalent to the IID_PPV_ARGS macro, and such a function wouldn't work with this macro either, I believe unless it's a trivial fix some wil helper might be more suitable.

kennykerr commented 1 year ago

Right, I wouldn't call this a bug in C++/WinRT. This mistake is somewhat prevalent in the Windows SDK, but it is a mistake and should be void**. I'm not opposed to adding support nonetheless as those headers can't be fixed now for fear of breaking the world but I'm not sure how easy that would be as C++/WinRT correctly assume void** on the bound function.

kennykerr commented 1 year ago

Something like this should do the trick, if someone would like to run with it:

namespace winrt::impl
{
    struct capture_decay
    {
        void** result;

        template <typename T>
        operator T** ()
        {
            return reinterpret_cast<T**>(result);
        }
    };

    template <typename T, typename F, typename...Args>
    int32_t capture_to(void**result, F function, Args&& ...args)
    {
        return function(args..., guid_of<T>(), capture_decay{ result });
    }
jonwis commented 1 year ago

Oh that's much nicer than the "figure out the type of the last parameter and use it" thing I was trying to do.

Do you think this is still appropriate for cppwinrt? WIL is fine too.

kennykerr commented 1 year ago

This seems like a very minor generalization, so I'm happy for it to live here.

kennykerr commented 1 year ago

Here you go: #1301