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

Fix gconstpointer fixup #779

Merged
merged 5 commits into from May 24, 2019
Merged

Fix gconstpointer fixup #779

merged 5 commits into from May 24, 2019

Conversation

fkrull
Copy link
Contributor

@fkrull fkrull commented May 21, 2019

Ran into this: I have an _equals and _hash that take gconstpointer and I had to change the *mut to *const in the generated code to make it compile.

This change feels like it should have unintended side effects, but also if you're dealing with gconstpointer, *const seems always correct.

@EPashkin
Copy link
Member

EPashkin commented May 21, 2019

@fkrull This change broke glib::DateTime because Shared types implement only ToGlibPtr<*mut _>,
can you show bad object definition's "glib_wrapper!" and generated hash ?

@fkrull
Copy link
Contributor Author

fkrull commented May 22, 2019

@EPashkin Here's the entire offending file. This is generated by this modified branch, but the only difference is the *const instead of *mut. I can dig out the error message, but I think it was just the opposite of what you described: that the type doesn't implement ToGlibPtr<*mut _>.

(The compiler helpfully suggested the *const implementation to me so I may have jumped to conclusions there.)

@EPashkin
Copy link
Member

Yes, Boxed types don't implement ToGlibPtr<*mut _>, so only boxed requires "*const" in fixup_special_functions.
Bad that detection logic for it is not compact.
You can set flag like is_boxed in https://github.com/gtk-rs/gir/blob/master/src/analysis/record.rs#L89-L91 and pass it all way down.

@fkrull
Copy link
Contributor Author

fkrull commented May 23, 2019

@EPashkin Ok, I've added a check on that flag which should fix glib.

@EPashkin
Copy link
Member

Can you fix pointer for RecordType::AutoBoxed too? This is second branch in https://github.com/gtk-rs/gir/blob/master/src/analysis/record.rs#L89-L91

@fkrull
Copy link
Contributor Author

fkrull commented May 24, 2019

@EPashkin Thanks for the hints. I hope that should be it now? glib generates as before and my tricky type got fixed up even without having to specify use_boxed_functions explicitly.

@@ -80,23 +80,21 @@ pub fn new(env: &Env, obj: &GObject) -> Option<Info> {
None => return None,
};

let use_boxed_functions = obj.use_boxed_functions;
let use_boxed_functions =
obj.use_boxed_functions || RecordType::of(&record) == RecordType::AutoBoxed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, rename variable to "is_boxed".

@EPashkin
Copy link
Member

@fkrull Good to hear that config not needed in your case.

@EPashkin
Copy link
Member

Thanks, I waiting CI just in case, then will do squash+merge

@EPashkin EPashkin merged commit f511aae into gtk-rs:master May 24, 2019
@fkrull fkrull deleted the fixup-gconstpointer branch May 29, 2019 17:26
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.

None yet

2 participants