Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritize RecordType::Refcounted over other record types #552

Merged
merged 1 commit into from
Feb 23, 2018

Conversation

EPashkin
Copy link
Member

as only this record type is referenced without mut

cc @GuillaumeGomez, @sdroege

@EPashkin
Copy link
Member Author

Added regen example

@EPashkin
Copy link
Member Author

Record GtkWidgetPath has both ref/unref and copy/free pairs, so detected as RecordType::Boxed before.
It usage has many const access and this breaks build (but maybe gtk-rs/glib#291 helps fix too)

@sdroege
Copy link
Member

sdroege commented Feb 23, 2018

Doesn't change anything for me and makes sense. If there is the possibility of refcounting, we should use it to prevent copies. The disadvantages of refcounted types exist already anyway (e.g. we can't make this Send without Sync) as C code might already make use of the refcounting.

It might make sense to expose the copy function independently though to be able to create a real copy.

@EPashkin
Copy link
Member Author

We already expose copy for shared(refcounted) records, try find gtk_widget_path_copy in regen example

@sdroege
Copy link
Member

sdroege commented Feb 23, 2018

Ok then :)

@GuillaumeGomez
Copy link
Member

Seems good to me as well! :)

@EPashkin
Copy link
Member Author

Thanks, for review

@EPashkin EPashkin merged commit 02e2302 into gtk-rs:master Feb 23, 2018
@EPashkin EPashkin deleted the prioritize_refcounted_record branch February 23, 2018 09:24
vhdirk pushed a commit to vhdirk/gir that referenced this pull request Jul 6, 2018
Prioritize RecordType::Refcounted over other record types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants