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

Implement configuration/detection for final types and use that inform… #694

Merged
merged 1 commit into from Jan 21, 2019

Conversation

Projects
None yet
3 participants
@sdroege
Copy link
Member

sdroege commented Jan 20, 2019

…ation

Final types are those that can't have any further subclasses, and as
such we don't need to generate IsA<_> bounds or NONE_XXX constants and
also don't need to generate a trait for them.

This replaces the previous "trait" configuration with "final_type",
which has a slightly wider meaning.

Show resolved Hide resolved src/analysis/bounds.rs Outdated
Show resolved Hide resolved src/config/gobjects.rs Outdated
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

IMHO main problem with final_type that we need set it in all "child" crates to works properly.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

IMHO main problem with final_type that we need set it in all "child" crates to works properly.

All child crates need to add the types to the manual section of their .toml already anyway, and for some types more configuration than that is already necessary. Check the Gir_Gst.toml for examples. So I think that's fine.

And if a child crate forgets to put this configuration there it won't generate broken bindings. It will only crate functions that are not as convenient to use as they could otherwise.

@sdroege sdroege force-pushed the sdroege:final-types branch from b8dc762 to 2a954ab Jan 21, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

Ok, implemented it like that

@sdroege sdroege force-pushed the sdroege:final-types branch from 2a954ab to bb43f38 Jan 21, 2019

Show resolved Hide resolved src/config/gobjects.rs Outdated
Show resolved Hide resolved src/library_postprocessing.rs Outdated
Show resolved Hide resolved src/library_postprocessing.rs Outdated
Show resolved Hide resolved src/analysis/object.rs Outdated

@sdroege sdroege force-pushed the sdroege:final-types branch from bb43f38 to 1da9f4b Jan 21, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

Updated, anything else needing changes?

@sdroege sdroege force-pushed the sdroege:final-types branch from 1da9f4b to ae5c8e2 Jan 21, 2019

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

@sdroege Thanks, can you change match *self.type_mut(tid) { to if-let too?
Note: I don't prefer not merge this before #667 so it may need conflict fixing.

@sdroege sdroege force-pushed the sdroege:final-types branch from ae5c8e2 to dc53c93 Jan 21, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

@sdroege Thanks, can you change match *self.type_mut(tid) { to if-let too?

Done. I disagree with that though, the old code would've caught inconsistencies that now would be silently ignored.

Note: I don't prefer not merge this before #667 so it may need conflict fixing.

I just checked and it merges fine apart from a minor merge conflict in a single import statement.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

if let can have else 😉 , I agree that will be safer in case function extension.

Implement configuration/detection for final types and use that inform…
…ation

Final types are those that can't have any further subclasses, and as
such we don't need to generate IsA<_> bounds or NONE_XXX constants and
also don't need to generate a trait for them.

This replaces the previous "trait" configuration with "final_type",
which has a slightly wider meaning.

@sdroege sdroege force-pushed the sdroege:final-types branch from dc53c93 to 122dcbf Jan 21, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

if let can have else wink , I agree that will be safer in case function extension.

Indeed, changed :) Sorry, was a long day!

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

@sdroege please check generated gio,
Cancellable now used without IsA, NONE_CANCELLABLE not generated.
Cancelable has no methods, while have GCancellableClass.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

Cancelable has no methods, while have GCancellableClass.

Why does it have no methods? g_cancellable_cancel(), etc? :) It's not a final type in every regard

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

Why does it have no methods? g_cancellable_cancel(), etc? :) It's not a final type in every regard

Ah, because it's marked as final type in the toml file. So that needs a change in the GIO toml to not mark it as final type (because it is not).

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 21, 2019

I'll send a PR for that once it's merged. We ideally get regens for everything once this merged, so maybe do that together with @GuillaumeGomez's #667

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

@sdroege You right, we just make wrong decision long ago and added "trait = false".

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

Hm, there many final types in gio, maybe better merge this before #667 and add separate PRs while its checkable.
@GuillaumeGomez You allow merge this PR?
This will cause single conflict in use, but may repeated on many commits on rebase.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 21, 2019

Yep, it's fine. I'll just fix conflicts on my PR.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 21, 2019

Then lets merge

@EPashkin EPashkin merged commit 4620f19 into gtk-rs:master Jan 21, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.