gir

gir copied to clipboard

after the switch to having impl in the arguments, a regression was introduced where such code is generated

Before:

fn uninstall<P: IsA<gio::Cancellable>>(
    &self,
    kind: RefKind,
    name: &str,
    arch: Option<&str>,
    branch: Option<&str>,
    progress: Option<&mut dyn (FnMut(&str, u32, bool))>,
    cancellable: Option<&P>,
) -> Result<(), glib::Error> {
    let progress_data: Option<&mut dyn (FnMut(&str, u32, bool))> = progress;
    unsafe extern "C" fn progress_func<P: IsA<gio::Cancellable>>(
        status: *const libc::c_char,
        progress: libc::c_uint,
        estimating: glib::ffi::gboolean,
        user_data: glib::ffi::gpointer,
    ) {
        let status: Borrowed<glib::GString> = from_glib_borrow(status);
        let estimating = from_glib(estimating);
        let callback: *mut Option<&mut dyn (FnMut(&str, u32, bool))> =
            user_data as *const _ as usize as *mut Option<&mut dyn (FnMut(&str, u32, bool))>;
        if let Some(ref mut callback) = *callback {
            callback(status.as_str(), progress, estimating)
        } else {
            panic!("cannot get closure...")
        };
    }
    let progress = if progress_data.is_some() {
        Some(progress_func::<P> as _)
    } else {
        None
    };
    let super_callback0: &Option<&mut dyn (FnMut(&str, u32, bool))> = &progress_data;
    unsafe {
        let mut error = ptr::null_mut();
        let _ = ffi::flatpak_installation_uninstall(
            self.as_ref().to_glib_none().0,
            kind.into_glib(),
            name.to_glib_none().0,
            arch.to_glib_none().0,
            branch.to_glib_none().0,
            progress,
            super_callback0 as *const _ as usize as *mut _,
            cancellable.map(|p| p.as_ref()).to_glib_none().0,
            &mut error,
        );
        if error.is_null() {
            Ok(())
        } else {
            Err(from_glib_full(error))
        }
    }
}

After

fn uninstall(
    &self,
    kind: RefKind,
    name: &str,
    arch: Option<&str>,
    branch: Option<&str>,
    progress: Option<&mut dyn (FnMut(&str, u32, bool))>,
    cancellable: Option<&impl IsA<gio::Cancellable>>,
) -> Result<(), glib::Error> {
    let progress_data: Option<&mut dyn (FnMut(&str, u32, bool))> = progress;
    unsafe extern "C" fn progress_func(
        status: *const libc::c_char,
        progress: libc::c_uint,
        estimating: glib::ffi::gboolean,
        user_data: glib::ffi::gpointer,
    ) {
        let status: Borrowed<glib::GString> = from_glib_borrow(status);
        let estimating = from_glib(estimating);
        let callback: *mut Option<&mut dyn (FnMut(&str, u32, bool))> =
            user_data as *const _ as usize as *mut Option<&mut dyn (FnMut(&str, u32, bool))>;
        if let Some(ref mut callback) = *callback {
            callback(status.as_str(), progress, estimating)
        } else {
            panic!("cannot get closure...")
        };
    }
    let progress = if progress_data.is_some() {
        Some(progress_func as _)
    } else {
        None
    };
    let super_callback0: &Option<&mut dyn (FnMut(&str, u32, bool))> = &progress_data;
    unsafe {
        let mut error = ptr::null_mut();
        let _ = ffi::flatpak_installation_uninstall(
            self.as_ref().to_glib_none().0,
            kind.into_glib(),
            name.to_glib_none().0,
            arch.to_glib_none().0,
            branch.to_glib_none().0,
            progress,
            super_callback0 as *const _ as usize as *mut _,
            cancellable.to_glib_none().0,
            &mut error,
        );
        if error.is_null() {
            Ok(())
        } else {
            Err(from_glib_full(error))
        }
    }
}

The cancellable.to_glib_none().0 should be cancellable.map(|p| p.as_ref()).to_glib_none().0, instead

The GIR definition

<method name="uninstall" c:identifier="flatpak_installation_uninstall" deprecated="1" deprecated-version="1.7.0" throws="1">
    <doc xml:space="preserve" filename="flatpak-installation.c" line="2124">This is an old deprecated function, you should use
#FlatpakTransaction and flatpak_transaction_add_uninstall()
instead. It has a lot more interesting features.

Uninstall an application or runtime.</doc>
    <doc-deprecated xml:space="preserve">Use flatpak_transaction_add_uninstall() instead.</doc-deprecated>
    <source-position filename="flatpak-installation.h" line="369"/>
    <return-value transfer-ownership="none">
        <doc xml:space="preserve" filename="flatpak-installation.c" line="2144">%TRUE on success</doc>
        <type name="gboolean" c:type="gboolean"/>
    </return-value>
    <parameters>
        <instance-parameter name="self" transfer-ownership="none">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2126">a #FlatpakInstallation</doc>
            <type name="Installation" c:type="FlatpakInstallation*"/>
        </instance-parameter>
        <parameter name="kind" transfer-ownership="none">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2127">what this ref contains (an #FlatpakRefKind)</doc>
            <type name="RefKind" c:type="FlatpakRefKind"/>
        </parameter>
        <parameter name="name" transfer-ownership="none">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2128">name of the app or runtime to uninstall</doc>
            <type name="utf8" c:type="const char*"/>
        </parameter>
        <parameter name="arch" transfer-ownership="none" nullable="1" allow-none="1">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2129">architecture of the app or runtime to uninstall; if
%NULL, flatpak_get_default_arch() is assumed</doc>
            <type name="utf8" c:type="const char*"/>
        </parameter>
        <parameter name="branch" transfer-ownership="none" nullable="1" allow-none="1">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2131">name of the branch of the app or runtime to uninstall;
if %NULL, `master` is assumed</doc>
            <type name="utf8" c:type="const char*"/>
        </parameter>
        <parameter name="progress" transfer-ownership="none" nullable="1" allow-none="1" scope="call" closure="5">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2133">the callback</doc>
            <type name="ProgressCallback" c:type="FlatpakProgressCallback"/>
        </parameter>
        <parameter name="progress_data" transfer-ownership="none" nullable="1" allow-none="1" closure="4">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2134">user data passed to @progress</doc>
            <type name="gpointer" c:type="gpointer"/>
        </parameter>
        <parameter name="cancellable" transfer-ownership="none" nullable="1" allow-none="1">
            <doc xml:space="preserve" filename="flatpak-installation.c" line="2135">a #GCancellable</doc>
            <type name="Gio.Cancellable" c:type="GCancellable*"/>
        </parameter>
    </parameters>
</method>

Adding it to the blockers for next release.

Maybe @MarijnS95 might have an idea about?

@sdroege Pinged me about code containing exactly that map in https://github.com/gtk-rs/gir/pull/1245#issuecomment-954856146 - don't know if anything was resolved around it?

The strange thing being that this is an Option<&P> which holds a borrow already - ~why do you need to call as_ref() on P: IsA?~ EDIT: What is it converting into?

What is it converting into? to the parent type; a gio::Cancellable. That's how all the subclassable type works right now

Why does it generate the correct code for all the same pattern in gio? :) That was one of the testcases when doing my changes recently.

No idea at all, if you can give me some tips where I should look at I can give it a try

The MR that broke it should be https://github.com/gtk-rs/gir/pull/1245.