Draft: Use Into<Option<_>> in argument position where applicable

[fengalin]

See the gir PR for details.

TODO

• rebase, update gir versions and re-generate when gir PR is merged

[sdroege]

We did that (or something very similar) at some point but then reverted it again because it made it hard to use in cases where there would be multiple possible trait impls, and the compiler errors were not very helpful. Is this not a concern anymore or does this only cover the clear cases?

[fengalin]

In my tests so far, I only encountered rare cases such as the one fixed in 8c09634, in response to the following error:

error[E0277]: the trait bound `Option<&str>: From<Option<&gio::glib::GString>>` is not satisfied
   --> gio/tests/dbus_peer.rs:34:13
    |
32  |         let connection = DBusConnection::new_future(
    |                          -------------------------- required by a bound introduced by this call
33  |             &socket_connection,
34  |             Some(&guid),
    |             ^^^^^^^^^^^ the trait `From<Option<&gio::glib::GString>>` is not implemented for `Option<&str>`, which is required by `Option<&gio::glib::GString>: Into<Option<&str>>`
    |
    = help: the following other types implement trait `From<T>`:
              `Option<&T>` implements `From<&Option<T>>`
              `Option<&mut T>` implements `From<&mut Option<T>>`
              `Option<T>` implements `From<T>`
    = note: required for `Option<&gio::glib::GString>` to implement `Into<Option<&str>>`
note: required by a bound in `DBusConnection::new_future`
   --> /home/fengalin/Projects/gtk-rs-core/gio/src/auto/dbus_connection.rs:947:20
    |
945 |     pub fn new_future<'a>(
    |            ---------- required by a bound in this associated function
946 |         stream: &(impl IsA<IOStream> + Clone + 'static),
947 |         guid: impl Into<Option<&'a str>>,
    |                    ^^^^^^^^^^^^^^^^^^^^^ required by this bound in `DBusConnection::new_future`

[sdroege]

error[E0277]: the trait bound Option<&str>: From<Option<&gio::glib::GString>> is not satisfied

Hm, why not and should we maybe add that? :)

In my tests so far, I only encountered rare cases

Can you highlight the others ones in the different PRs/MRs together with the compiler error?

[fengalin]

error[E0277]: the trait bound Option<&str>: From<Option<&gio::glib::GString>> is not satisfied
Hm, why not and should we maybe add that? :)

Take a look at this playground for an illustration. AIUI, the problem is not really that Option<&str> can't be built from Option<&gio::glib::GString>, but rustc doesn't seem to be able to handle this behind an impl Into<_> bound. A way to handle this would be the 3d case, but it adds more inconvenience than it solves imo. However:

In my tests so far, I only encountered rare cases
Can you highlight the others ones in the different PRs/MRs together with the compiler error?

In the 3 main repositories, that was the error from above and a couple of call sites in the gtk4 examples.

I pushed a gst-plugins-rs branch patched to use the 3 repos and got only one LazyLock call site which needed a fix. Also I pushed a preview in another branch of what to expect if we'd used this usability improvement throughout the code (based on a previous iteration and not complete: in particular, this doesn't take advantage of the availability of Into<Option<_>> args in gtk-rs-core nor in gtk4).

Edit: there is also the last commit of this PR ("pango: manual impl for some functions ", see commit message for details), but that's not something that affects users.

Edit2: note that I didn't implement Into<Option<_>> for Option<AsRef<_>> cases, which cause annoying problems indeed, like in the playground above.

Edit 3: updated the cases which needed an updated (missed gtk4 examples).

[sdroege]

That Some(&stringly_value) doesn't compile anymore in many cases and requires an as_str() or otherwise an explicit conversion to &str seems a bit ugly. AFAIU that's because previously this was covered by auto-deref but that doesn't trigger anymore in the generic context? It feels like I created a rustc issue about that long ago.

These are all cases of impl Into<Option<impl IntoGStr>> or impl Into<Option<&str>> I assume? One could add a new IntoOptionStr trait or so for this but that seems more confusing.

One concern I have is that people might do as_str() for the impl IntoGStr case, while as_gstr() would be more efficient but less known.

Otherwise this seems fine. The same problem with as_str() also exists already on glib::GString if you want to match on them, for example. The compiler errors are also less bad than I remember them in the past.

@bilelmoussaoui Do you have opinions on this?

[fengalin]

I agree this case is suboptimal. Now that the generator implementation has settled down, I can take some time to figure out if something could improve this use case.

Worse case scenario, we could remove support for Into<Option<& T>> and keep the others which don't cause problems afaict: Into<Option<T>> and Into<Option<IsA<T>>.

[sdroege]

Worse case scenario, we could remove support for Into<Option<& T>> and keep the others which don't cause problems afaict: Into<Option<T>> and Into<Option<IsA<T>>.

impl Into<Option<impl IsA<T>>> AFAIU already has the problem right now with Option<impl IsA<T>> alone, so yes, that doesn't make things worse :)

Considering the minimal amount of changes you had to do and it only affecting strings, I'm not sure keeping it like this is really a problem.