Support composable constructors

[Alovchin91]

Suggestion

Hi,

I'm probably trying to do something forbidden 😅 but I think this is still a valid use case for the WinRT type system.

I'm looking at implementing a simple ViewModel with Rust that can then be used from a C# UI app.

I generate bindings as follows:

windows_bindgen::bindgen([
    "--in",
    "winmd/Microsoft.UI.Xaml.winmd",
    metadata_dir.join("Windows.winmd").to_str().expect("metadata_dir"),
    "--out",
    "src/microsoft.rs",
    "--filter",
    "Microsoft.UI.Xaml.Data.INotifyPropertyChanged",
    "Microsoft.UI.Xaml.Data.PropertyChangedEventHandler",
    "Microsoft.UI.Xaml.Data.IPropertyChangedEventArgs",
    "Microsoft.UI.Xaml.Data.PropertyChangedEventArgs",
    "--config",
    "implement",
])
.inspect_err(|err| eprintln!("bindgen failed: {err}"))
.ok()?;

And then I implement the INotifyPropertyChanged interface by saving the handlers into a hash map:

#[implement(ViewModels::MainWindowViewModel, INotifyPropertyChanged)]
struct MainWindowViewModel<'a> {
    event_handlers: RefCell<HashMap<i64, PropertyChangedEventHandler>>,
    next_event_registration_token: AtomicI64,
    button_text: RefCell<&'a str>,
    _phantom_data: PhantomData<&'a ()>,
}

impl INotifyPropertyChanged_Impl for MainWindowViewModel_Impl<'_> {
    fn PropertyChanged(&self, handler: Option<&PropertyChangedEventHandler>) -> Result<EventRegistrationToken> {
        let handler = handler.ok_or(Error::from(E_INVALIDARG))?;
        let event_registration_token = self.next_event_registration_token.fetch_add(1, Ordering::Relaxed);
        self.event_handlers
            .borrow_mut()
            .insert(event_registration_token, handler.clone());
        Ok(EventRegistrationToken {
            Value: event_registration_token,
        })
    }

    fn RemovePropertyChanged(&self, token: &EventRegistrationToken) -> Result<()> {
        self.event_handlers
            .borrow_mut()
            .remove(&token.Value)
            .ok_or(Error::from(E_INVALIDARG))?;
        Ok(())
    }
}

The issue is that, when firing handlers, I need to pass them an instance of PropertyChangedEventArgs. But the generated bindings don't contain a constructor for it, so there's no way for me to create a new instance:

impl ViewModels::IMainWindowViewModel_Impl for MainWindowViewModel_Impl<'_> {
    fn OnButtonClick(&self) -> Result<()> {
        self.button_text.replace("Button clicked!");
        let iface: InterfaceRef<'_, IInspectable> = self.as_interface_ref();
        for handler in self.event_handlers.borrow().values() {
            handler.Invoke(iface.into(), PropertyChangedEventArgs::new("ButtonText"));
                           ^^^^^^^^^^^^                            ^^^^^^^^^^^^^^^^^
                           |                                       |
                           not sure about this                     this does not exist !!!
        }
        Ok(())
    }
}

Am I missing something? Or are events and event args not supported too?

Please ignore the WinUI use case. Events are a part of the WinRT type system, so I believe they should be available to the users of windows-rs.

Thank you!

[kennykerr]

Events are fully supported. Whether they work in this case depends on the API. I'll take a look at this repro.

Related: #3157 #3244

[kennykerr]

OK, so the immediate issue here is that the PropertyChangedEventArgs unnecessarily uses type composition rather than simply providing a constructor directly. This is something I can add support for in Rust for compatibility.

Technically, it has a Composable factory rather than an Activatable factory.

[Alovchin91]

Thank you for such a quick response!

Do I understand correctly that this means that in the current version I would essentially need to #[implement(PropertyChangedEventArgs)] on my custom struct for this type to work?

[kennykerr]

To call that constructor you need to use IPropertyChangedEventArgsFactory and its CreateInstance method but that's not very easy to do at present.

[Alovchin91]

FYI as a workaround for now this seems to work:

#[implement(PropertyChangedEventArgs)]
struct MyPropertyChangedEventArgs {
    property_name: HSTRING,
}

impl MyPropertyChangedEventArgs {
    fn new<T: AsRef<str>>(property_name: T) -> Self {
        MyPropertyChangedEventArgs {
            property_name: HSTRING::from(property_name.as_ref()),
        }
    }
}

impl IPropertyChangedEventArgs_Impl for MyPropertyChangedEventArgs_Impl {
    fn PropertyName(&self) -> Result<HSTRING> {
        Ok(self.property_name.clone())
    }
}

I guess that's because PropertyChangedEventArgs is an unsealed class so I'm basically extending it.

[kennykerr]

The trouble is that PropertyChangedEventArgs is a class that you don't own and thus aren't supposed to implement at all, like say an interface. Implementations can make assumptions about the internal layout of those types that you may not be aware of. Anyway, #3246 adds support for composable constructors so you should be able to call the real PropertyChangedEventArgs constructor as you can in say C++ or C#.

[Alovchin91]

@kennykerr This works as expected, thank you very much! 🥇

I've noticed however that the factory function is public:

#[doc(hidden)]
pub fn IPropertyChangedEventArgsFactory<
    R,
    F: FnOnce(&IPropertyChangedEventArgsFactory) -> windows_core::Result<R>,
>(
    callback: F,
) -> windows_core::Result<R> {
    static SHARED: windows_core::imp::FactoryCache<
        PropertyChangedEventArgs,
        IPropertyChangedEventArgsFactory,
    > = windows_core::imp::FactoryCache::new();
    SHARED.call(callback)
}

This seems like an implementation detail so should probably be hidden from the user? 🤔

It's also a bit inconsistent that the constructor is named CreateInstance instead of new like for the EventHandler, but I suppose that's because a class can have both a composable factory and an activation factory (a parameterless constructor?) simultaneously?

[kennykerr]

#3261 for the factory functions - good catch!

Regarding new vs CreateInstance, the name new is only ever used for what we call "default constructors" in C++ and C#. Those are unnamed in metadata and the name "new" is natural n Rust and not going to collide with anything from metadata. Any other name is taken straight from metadata and must be used since Rust doesn't support overloads. API authors can provide more specific names but CreateInstance is what MIDL will generate by default. Here's an example of overriding the name:

https://github.com/microsoft/windows-rs/blob/master/crates/tests/winrt/constructors/src/metadata.idl

[Alovchin91]

You, sir, are an absolute hero! Thank you so much for addressing this!