signals: work-around ObjectType shadowing

[Mstrodl]

Related to #772

This is needed because use glib::prelude::* will not pull in ObjectType (even though we only need the trait in scope) because it's shadowed.

Another way to fix this is by using a fully-qualified path to ObjectType at the call-site (happy to submit another PR for that if that's preferred, I do have one somewhere...), but this is more consistent with what's already being done.

[bilelmoussaoui]

It is shadowed by what?

[Mstrodl]

It is shadowed by what?

libostree defines an ObjectType of its own. It means something completely different (enum of types of objects stored in an ostree repository)

[bilelmoussaoui]

I think a better solution would be for ostree to rename the enum to something else

[sdroege]

In the bindings, not in the C library (where it would be a breaking change). But also I think we should generally import traits as use FooTrait as _ and do the same thing in the preludes too.

[Mstrodl]

I think a better solution would be for ostree to rename the enum to something else

That would be a breaking change to a library that has existed for a long time... Personally, I'd prefer if gir were more explicit and use fully-qualified paths where possible rather than generating all these use statements, but as the previous issue touches on, that's a big undertaking, and would be a little fragile for now unless everything was converted. Everywhere else, it's not a problem because it's qualified: OstreeObjectType.

gir is already doing something similar to work around the collision with GtkBox and std::boxed::Box (see #206 (comment)), which is why I went with this approach:

gir/src/analysis/signals.rs

Line 106 in 61370e8

imports.add("std::boxed::Box as Box_");

[Mstrodl]

In the bindings, not in the C library (where it would be a breaking change). But also I think we should generally import traits as use FooTrait as _ and do the same thing in the preludes too.

Happy to swap use glib::object::ObjectType as ObjectType_ out with use glib::object::ObjectType as _ if you think that would be cleaner.

[sdroege]

Yeah I completely agree with all that :) It "just" needs someone to untangle the code.

Happy to swap use glib::object::ObjectType as ObjectType_ out with use glib::object::ObjectType as _ if you think that would be cleaner.

Both seems fine to me and I'm not sure why @bilelmoussaoui was against that. As you say, we do the same for GtkBox and std::box::Box already.

As this is a trait and never used by name, as _ seems cleaner though.

[bilelmoussaoui]

I would do the change in prelude then.

I am against this change because it confuses tools like rust analyzer. I wouldn't do what was done 9 years ago with Box and just fully qualify the std usages without importing the type.

[sdroege]

I am against this change because it confuses tools like rust analyzer

If use X as _ confuses rust-analyzer then that's a bug in rust-analyzer

[bilelmoussaoui]

Sure. But anyways, the change should be done on prelude as adding more imports to the generated code makes it more fragile to changes in prelude or where the traits are defined.

[sdroege]

That's a good point, I missed that this adds an import instead of just renaming it. Going with gtk-rs/gtk-rs-core#781 is probably a better idea indeed.

That would mean changing gir so that auto::traits has pub use X as _; lines and then doing the same for the prelude and subclass::prelude modules for extension traits and other traits that are usually not named directly (i.e. the subclass FooImpl traits should be exported by name still).

[Mstrodl]

Bear with me here just because I don't have a good knowledge of the gtk-rs ecosystem, I want to make sure I understand:

Sounds like consensus is to rename the reexports in glib's prelude to _'s? Looks like that's here and easily fixed: https://github.com/gtk-rs/gtk-rs-core/blob/89254b3e5a594867cc917e83987df43c5191a51f/glib/src/prelude.rs#L6-L14

That should fix the issue I described with libostree.

Separate from that, for consistency's sake and so this doesn't become an issue with other traits, you'd like to modify other projects in the ecosystem to do the same thing (gir should generate anonymous exports in auto::traits, subclass::prelude, and prelude)?

This is definitely a breaking change, but it's probably not a big deal because very few people are likely referencing those traits by name, right?

I can submit a PR for the glib change.

[sdroege]

Yeah that's correct. Just updating the glib::prelude and glib::subclass::prelude as a first step would be OK

This is definitely a breaking change

That's fine, it would be released with the next breaking release in summer. You probably need this solved earlier though, in which case we could consider a temporary workaround like this here (just make it as _) for the current stable release series.

[Mstrodl]

Ok. I replaced the commit with one that imports as _

[Mstrodl]

@sdroege So, it actually looks like glib itself makes extensive use of having the prelude type names in-scope. I could explicitly include all the traits in glib where they're needed, but it is somewhat convenient to have them in a prelude still for commonly-used stuff.

Another option would be to make a separate anonymous_prelude module that reexports everything as _ for external consumers. This has the advantage of not breaking existing code and not requiring a bunch of use statements to be added to glib proper. Alternatively, I could do the other way around, repurposing prelude as _ exports and create a new prelude (possibly crate-local?) which glib itself uses when it wants trait names in-scope.

Another thing I could do is to have prelude continue exporting by name, but also anonymously: pub use crate::object::{ObjectType, ObjectType as _}. If desired, this could instead be pub use crate::object::ObjectType as _; pub(crate) use crate::object::ObjectType;

I do wish that rust supported something like use glib::prelude::* as _...

Let me know what you prefer!

[sdroege]

That's tricky. Outside glib, do you think any of the traits make sense to be re-exported from the prelude by name?

[Mstrodl]

That's tricky. Outside glib, do you think any of the traits make sense to be re-exported from the prelude by name?

I don't think so? When I wrote some rust gtk4 stuff, I'm pretty sure those trait implementations came automatically from macros, but it's been a while so I really can't remember.

[sdroege]

I think then the glib prelude shouldn't re-export them by name. The prelude is meant for outside code.

@bilelmoussaoui Do you think any of the prelude traits should be available by name?