Bug:Compiling Error with C++20, inheriting of winrt::implements

ifgotmoney commented 4 months ago

Version

2.0.240405.15

Summary

Hello community,

in my WinUI3 project (C++/WinRT), in order to use winrt::weak_ref<> & get_weak() for events. I defined a base class as below: class IDrawable: public winrt::implements<IDrawable, winrt::Windows::Foundation::IInspectable> and a "derived" class as below: class FlowLine : public winrt::implements<FlowLine,IDrawable,winrt::Windows::Foundation::IInspectable>

this pattern works fine for me with C++17 in Visual Studio Community 2022, however, when I try to change to C++20, the same code can not be compiled, and I got errors during compiling as below 'to_abi': no matching overloaded function found

any thoughts?

Reproducible example

//using a winui3 desktop app template C++/WinRT, define two classes as below, both of them are not runtimeclass, just inheriting from winrt::implements<>, in order to use winrt::weak_ref<> template for these classes.

// class A
class Aclass : public winrt::implements<Aclass,winrt::Windows::Foundation::IInspectable>
{
public:
    Aclass();
    virtual void SayHello()=0;
}

//class B
class Bclass : public winrt::implements<Bclass,Aclass,winrt::Windows::Foundation::IIspectable>
{
public:
     Bclass();
     virtual void SayHello() override;
}

Expected behavior

these code has been compiled sucessfully with Visual Studio Community 2022 with C++17, and the app works as expected for me. however, the same code can not be compiled with C++20, I expected to compile these code sucessfully with C++20 as I dont change even one character.

Actual behavior

Error during Compiling: C2672 'to_abi': no matching overloaded function found

Additional comments

No response

sylveon commented 4 months ago

You can only have one instance of winrt::implements in a class hierarchy. In Bclass, you have two. Just directly inherit from Aclass (class Bclass : public Aclass)

ifgotmoney commented 4 months ago

You can only have one instance of winrt::implements in a class hierarchy. In Bclass, you have two. Just directly inhereit from Aclass (class Bclass : public Aclass)

I tried this way, but it doesn't work.

my code just works fine with C++17, but not work with C++20, I wonder know if c++/winrt is compatible with C++20;

sylveon commented 4 months ago

C++/WinRT is compatible with C++20, what you are seeing is that this code should've been rejected in C++17 mode but wasn't (and in fact Clang does reject it in C++17). This MSVC specific compiler bug was fixed with C++20, so the code is now properly rejected in C++20 mode.

class Bclass : public Aclass works properly for me. Perhaps we need more context to evaluate why this won't work for you?

Alternatively, you can have only the most derived classes make use of winrt::implements, or, if you only need weak reference functionality, completely forgo winrt::implements and use std::shared_ptr (which can be converted to std::weak_ptr).

Another alternative is making use of WinRT composition with winrt::composable and winrt::composing marker structs, but it's most likely overkill for your use case.

ifgotmoney commented 4 months ago

C++/WinRT is compatible with C++20, what you are seeing is that this code should've been rejected in C++17 mode but wasn't (and in fact Clang does reject it in C++17). This MSVC specific compiler bug was fixed with C++20, so the code is now properly rejected in C++20 mode.

class Bclass : public Aclass works properly for me. Perhaps we need more context to evaluate why this won't work for you?

Alternatively, you can have only the most derived classes make use of winrt::implements, or, if you only need weak reference functionality, completely forgo winrt::implements and use std::shared_ptr (which can be converted to std::weak_ptr).

Another alternative is making use of WinRT composition with winrt::composable and winrt::composing marker structs, but it's most likely overkill for your use case.

thank you for the reply @sylveon .

the reason I want to use winrt::weak_refrather than std::weak_ptr is for the winrt::event registering. with the winrt::weak_ref I can simply register an event handler as below:

... m_xxrevoker=eventsource.OnMousePressed(winrt::auto_revoke,{get_weak(), &eventrecipient::eventhandler}) ...

with above code, there will be no lifetime issue of thispointer in my eventhandler, if using std::shared_ptr and std::weak_ptr, the code will be a little bit complicated. my app has dedicated thread to handle mouse input, and the eventhandlers need to be registered and revoked many times, and the user may delete the event recipient any time, so I want to use winrt::weak_refto make the code simple as said https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/weak-references.

in my code, the Aclass is actually an abstract class, I wonder what's the right way to write an user-defined interfacejust like IInspectable ``IStringable..etc. should I define Aclass as an interface in an IDL file?

sylveon commented 4 months ago

the reason I want to use winrt::weak_ref rather than std::weak_ptr is for the winrt::event registering. with the winrt::weak_ref I can simply register an event handler as below:

You can do the same with std::shared_ptr, just use weak_from_this(), see #1330.

Also, if Aclass is strictly an interface, you can define it as such:

MIDL_INTERFACE("SOME-GUID") IAclass : IUnknown
{
    virtual void SayHello() = 0;
    virtual std::string GetString() = 0; // no need to conform to COM interface restrictions
};

And then do like this

class Bclass : public winrt::implements<Bclass, IAclass>

microsoft / cppwinrt