Support mutable method arguments with dbus_interface macro #311

Closed zeenix closed 1 year ago

In GitLab by @vanadiae on Dec 27, 2022, 18:18

Currently it's possible to directly declare a method argument as mutable when using the dbus_proxy macro, but when using dbus_interface it gives an error, that disappears when dropping the dbus_interface macro:

error: expected expression, found keyword `mut`
   --> src/example.rs:189:23
    |
189 |     async fn t(&self, mut s: String) -> fdo::Result<()> {
    |                       ^^^ expected expression

error: expected expression
   --> src/example.rs:189:23
    |
189 |     async fn t(&self, mut s: String) -> fdo::Result<()> {

with example code

struct Test;
#[dbus_interface(name = "org.example.Test")]
impl Test {
    async fn t(&self, mut s: String) -> fdo::Result<()> {
        s.push_str("foo");
        Ok(())
    }
}

Of course in this case it's possible to clone and shadow the argument, but it's less convenient and wastes memory (in the case where borrowed arguments are used).

This should be fairly trivial to fix.

In GitLab by @vanadiae on Dec 28, 2022, 01:01

I didn't know at all how macro code worked before digging into this particular one, with syn and quote crates docs to help, but after looking a bit at the macro code (in iface.rs), it looks like the issue ought to be between lines 626 and 654 here, or any later use of the returned args_from_msg, all_args. I don't see how the quote! interpolation line 640 could cause this expected expression error on the mut keyword, since here args is only the "pat" part of the PatType, which at the end should be a PatIdent, hence including the mut keyword. At that point, interpolating this line should result in let (mut s,): (String,) = … which is a perfectly valid variable binding here as far as I can tell. Line 210 looks like it would be invalid though after interpolating, as we'd get &(mut s,), …, where the mut is probably unexpected/invalid, though here it's for signals so this code isn't even run during macro compilation. Line 370 has the same issue, where mut s is passed as argument, so it's probably the line that breaks things since it's the else branch, after treating properties and signals.

Now I've never done macro stuff before, so this may very well be totally wrong, but at least that's what I found. I don't have zbus checked out right now, so I'll see if I can find time in the next few days to test it (or if you want to try it out if my reasoning looks alright) and send a MR.

Thanks for looking into this.

don't have zbus checked out right now, so I'll see if I can find time in the next few days to test it (or if you want to try it out if my reasoning looks alright) and send a MR.

If I go through your explaination above and checked in the code for it to be correct, I might as well do the fix while I'm there if you got everything right. :) Best way is to just do it and add a testcase (or adjust an existing one) to be sure your fix works.

In GitLab by @vanadiae on Dec 28, 2022, 19:07

Sent in !625 :)

mentioned in commit 4ccd8a6aae52ca501172b4dfff462e47adbf7450

dbus2 / zbus-old

Support mutable method arguments with dbus_interface macro #311