rustc_hir: add Expr! pattern macro and try it out in a couple places.

[eddyb]

Came across more usecases for pattern aliases today, and realized we can use macros instead!
EDIT: I just checked, and pattern macros seem to be stable in 1.0? But they wouldn't have been scoped.

(click for backstory - TL;DR: pattern-matching Ty instead of TyKind)

---

I wanted to be able to write something like this, so that when the type doesn't match, it's printed (as opposed to just assert!(ty.is_ref() || ty.is_ptr())):

assert_matches!(ty.kind, ty::Ref(..) | ty::RawPtr(..));

But that would print ty.kind, instead of ty, so I'd want to match on ty instead:

assert_matches!(ty, ty::Ref!(..) | ty::RawPtr!(..));

The other example I came up with was ty::Ref!(_, _, ty::Str!()) (for &str).

---

Here that idea is applied to HIR (and just Expr in particular), with a few patterns across the codebase replaced, to get a first impression of what can be done with it.

In the first commit I've included two examples (rustc_lint::builtin and rustc_typeck::astconv), which I found more interesting, while the second commit is more of a random set of replacements.

---

The definition of hir::Expr!, if we elide $crate::hir:: paths, looks like this:

pub macro Expr($variant:ident $($rest:tt)*) {
    Expr { kind: ExprKind::$variant $($rest)*, .. }
}

At its most basic, this allows writing e.g. hir::Expr!(Lit(_)) in a pattern.

But the real benefit comes from making nesting straightforward, e.g.:

hir::Expr!(Unary(hir::UnOp::UnNeg, hir::Expr!(Lit(lit))))

Also, you may have noticed the definition is technically more flexible than it needs to be.
This allows mentioning other fields as well, e.g. hir::Expr! { Path(_), hir_id: path_hir_id }.
I'm not attached to the syntax, but I think this is a neat ability.

The most complex pattern in this PR, after rustfmt, looks like this:

hir::Expr!(
    MethodCall(
        _,
        _,
        [hir::Expr!(Call(hir::Expr! { Path(ref qpath), hir_id: path_hir_id }, _)), ..],
    )
)

---

This probably qualifies for a @rust-lang/compiler design meeting discussion, but this is a rabbit hole I don't want to spend much time on, maybe @Centril or others are interested in taking over?

[rust-highfive]

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

[nikomatsakis]

Interesting. This could affect some of the "chalk-ty" questions. In particular, I've been wondering about whether to use "a few general variants" or "lots of fine-grained variants". The latter are annoying most of the time but potentially a more space-efficient representation, and occasionally convenient. Pattern aliases might "sort of" side-step the problem, at the cost of being a weird pattern with worse error messages.

[bors]

☔ The latest upstream changes (presumably #67476) made this pull request unmergeable. Please resolve the merge conflicts.

[petrochenkov]

My knee-jerk reaction is "I don't like this".
I see macros as generally evil and something that shouldn't be used unless it brings some large benefit, which I don't see here.

Fragments with Expr { kind: hir::ExprKind:: ... replaced by the macro look somewhat reasonable, but others like hir::ExprKind::Block(block, None) -> hir::Expr!(Block(block, None)) just bring macro calls where was none previously.

[petrochenkov]

$crate -> crate here and one line above, macros are fully hygienic and don't need $crate.

[eddyb]

I wanted to be conservative, presumably I can even write Expr without the full module path.

[petrochenkov]

Perhaps if there's a convention, and these macros are used consistently, and everyone knows what hir::Thing!(...) or ast::Thing!(...) means it will be better.
But I'd still want to see ThingKind::Foo(...) instead of hir::Thing!(Foo(...)) in code.

We probably need to estimate how often Thing { kind: ThingKind { ... }, ... } is actually encountered in practice and decide whether the macros worth it depending on that.

(Also, these macros can be generated with derives instead of being written manually for each struct.)

[Centril]

I sorta feel the same as @petrochenkov and don't think I would be game to implement use of this myself. Beyond making code more cryptic, remember that macro usage like this angers rust-analyzer in general, making interactions with IDEs worse.

[flodiebold]

macro usage like this angers rust-analyzer in general, making interactions with IDEs worse

We don't expand macros in patterns in rust-analyzer right now, but I think implementing it would mostly be trivial 🙂 Support for macros 2.0 is non-existent though, and completion within the macro call would be a problem.

[eddyb]

We probably need to estimate how often Thing { kind: ThingKind { ... }, ... } is actually encountered in practice and decide whether the macros worth it depending on that.

To be clear, that's not the usecase I'm targeting, because that sort of code is avoided, and instead we rely on nested matches in a bunch of places.
And for Ty you can't write the pattern yourself even if you wanted to.

However, I also understand if the people working with the relevant data structures would prefer to avoid macros, and wait until we have pattern aliases to do any ergonomic overhauls (if ever).

I have unrelated projects which use indices for interning, not references, and so pattern aliases wouldn't help there, so I might try the macro approach there.

Anyway, I mostly wanted to bring it up in case we overlooked this sort of thing being possible at all, not to champion it. I'll leave the PR open for maybe another week but only so more people have a chance to see it.

[nagisa]

why not use pat fragment?

[eddyb]

Because then people would have to write hir::ExprKind::Foo instead of Foo, inside the macro arguments.
And also this allows some trickery like matching on the hir_id (although maybe that should be done with @ instead of this EDIT: nevermind, @ needs a binding on one side, it's not a conjunction of patterns).

[pnkfelix]

Discussed at T-compiler meeting.

Of those present, general feeling was that no one was enthusiastic about making this change. We will leave this open for now, with the intent to close in a week or so unless a champion comes forward.

[petrochenkov]

We will leave this open for now, with the intent to close in a week or so unless a champion comes forward.

A week has passed.

[eddyb]

Note to self: don't delete the branch in case someone comes across this later.