Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Also require ToGlibPtr<_, *const _> for the IsA<_> trait #249

Closed
wants to merge 1 commit into from
Closed

Also require ToGlibPtr<_, *const _> for the IsA<_> trait #249

wants to merge 1 commit into from

Conversation

sdroege
Copy link
Member

@sdroege sdroege commented Nov 12, 2017

The macro is implementing both anyway, and we might need to be able to
generate const pointers for various FFI API.

@zeenix

The macro is implementing both anyway, and we might need to be able to
generate const pointers for various FFI API.
@sdroege
Copy link
Member Author

sdroege commented Nov 12, 2017

This now breaks type inference in various places. Do we want that?

@zeenix
Copy link
Contributor

zeenix commented Nov 12, 2017

I sent a patch to fix this upstream in libosinfo so let's see if that get's merged instead:

https://www.redhat.com/archives/libosinfo/2017-November/msg00008.html

@EPashkin
Copy link
Member

I don't found any usage of *const in generated code.
In manual code it seems usable (if works) only in sourceview/src/style.rs (not checked gstreamer-rs).
So seems mostly usage lies in glib.

@EPashkin
Copy link
Member

@zeenix Can you show example of function with const that generated bad way?

@zeenix
Copy link
Contributor

zeenix commented Nov 12, 2017

@EPashkin see the link to my patch email above for such functions in upstream. Do you want me to paste the generated bad code?

@EPashkin
Copy link
Member

@zeenix Yes, I on windows, so I can't generate it easily, so please, show me bad code.

@EPashkin
Copy link
Member

.. with part of .gir-file for this function.

@EPashkin
Copy link
Member

Found working version for one function (with restore constness in sys):

    pub fn osinfo_install_config_param_get_name(config_param: *const OsinfoInstallConfigParam) -> *const c_char;

    fn get_name(&self) -> Option<String> {
        unsafe {
            let stash:Stash<*mut _, _> = self.to_glib_none();
            from_glib_none(ffi::osinfo_install_config_param_get_name(stash.0 as *const _))
        }
    }

Theoretically we can change gir to produce this for &self but better find better way.

@EPashkin
Copy link
Member

@zeenix Gir updated to fix your problem, please check if it really helps.

@zeenix
Copy link
Contributor

zeenix commented Dec 18, 2017

@EPashkin thanks so much. The patch to libosinfo was in the end accepted actually so it's not that much an issue anymore but good to have the fix for existing libosinfo releases.

@zeenix
Copy link
Contributor

zeenix commented Dec 18, 2017

I tried to quickly test but there are currently other issues I'm facing with libosinfo-rs crate. I'll try to check later but feel free to close this issue.

agx pushed a commit to agx/libosinfo-debian that referenced this pull request Sep 17, 2018
Use of 'const' parameter on object parameters is redundant, inconsistent
(both internally and against other GObject libraries) and currently breaks
the low-level Rust binding generator:

gtk-rs/glib#249

Signed-off-by: Zeeshan Ali <zeeshan@kinvolk.io>
@sdroege
Copy link
Member Author

sdroege commented Jun 19, 2019

Outdated and not really needed anymore

@sdroege sdroege closed this Jun 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants