remove need to have last bracket in match_class! {} macro

[sylbeth]

I commented on this on my previous PR, so I wanted to remove the need for it. I pr'd one approach, but the other one is the following:

// Shadowed fallback var.
($subject:ident, $(_ => $block:expr $(,)?)?) => {{
    $($block)?
}};

I also wanted to comment on this comment that is still on the macro, are there any plans for it? Why was it left as a comment?

/*
// If we want to support `var @ _ => { ... }` syntax, use this branch first (to not match `_` as type).
($subject:ident, $var:ident @ _ => $block:expr $(,)?) => {{
    let $var = $subject;
    $block
}};
 */

The reasons I'd advocate for removing the need for the last branch is because there are many use cases in which that bracket is just adding more verbosity. My concern is what would happen in a match that returns an int but no last bracket is used, then, what is the compile error? The one that appeared during testing is an error on the macro definition itself.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1246

[Bromeon]

I also wanted to comment on this comment that is still on the macro, are there any plans for it? Why was it left as a comment?

No plans for now, but we can leave it for some time until we see how match_class! is going to be used.

---

My concern is what would happen in a match that returns an int but no last bracket is used, then, what is the compile error? The one that appeared during testing is an error on the macro definition itself.

You might be able to turn this into a type-system error by explicitly writing something like:

let () = $expr;

Clippy should allow this as per rust-lang/rust-clippy#9048, but I haven't tested. Otherwise

#[allow(clippy::let_unit_value)]
let _unit_value: () = $expr;

[sylbeth]

No plans for now, but we can leave it for some time until we see how match_class! is going to be used.

Gotcha!

let () = $expr;

Huh? How could that happen when no rule matches that afaik?

[Bromeon]

We should definitely not support this.

If a fallback branch is needed, then _ => is totally acceptable to write.

[sylbeth]

Perfect

[Bromeon]

It seems like the case where the ? is present is already handled, no?

Can this be written differently maybe -- to also outlaw the 3 syntax (see previous comment)?

[sylbeth]

Yeah! That's what I was requesting to know, I already had a solution.

[Bromeon]

Huh? How could that happen when no rule matches that afaik?

The things I can imagine are:

• assign the whole macro to a unit type, but not sure if that's compatible with the "muncher" approach
• if last branch is missing, expect unit type from previous branches -- but again hard with sequential iterative processing

We need to find a way that's syntactically consistent and intuitive, otherwise I'd rather have explicit _ => {}.

[sylbeth]

Right now it's either:

• Returns unit type, you can skip the _ => branch
• Returns non-unit type, you need to provide a fallback branch

[Bromeon]

Added a commit to test the empty match_class! statement, which is just a special case of this syntax. Also cleaned up some other parts (like the comment you suggested).

Generally looks good, you can squash (feel free to remove my authorship), I'll give some others the chance to review 👍

[sylbeth]

Alright! Should we document it on the macro's doc comment?

[Bromeon]

Yes, maybe update the sentence stating that the fallback is required -> only required if the arms have a type other than ().

The code can probably be left as-is 🤔

[sylbeth]

Should be all good now unless there's some issue with the documentation wording.