Accept interpolated patterns in trait method parameters

[petrochenkov]

Permit this, basically

macro_rules! m {
    ($pat: pat) => {
        trait Tr {
            fn f($pat: u8) {}
        }
    }
}

it previously caused a parsing error during expansion because trait methods accept only very restricted set of patterns during parsing due to ambiguities caused by anonymous parameters, and this set didn't include interpolated patterns.

Some outdated messages from "no patterns allowed" errors are also removed.

Addresses #35203 (comment)

[rust-highfive]

r? @eddyb

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

[eddyb]

LGTM. r? @nikomatsakis

[durka]

So is this also the "make #35203 a hard error" PR (skipping the deny-by-default step)?

[petrochenkov]

@durka
In practice #35203 == mut IDENT (according to crater runs) and mut IDENT is still reported as a warning.

The only patterns transitioning from warning to error are &IDENT and &&IDENT, other patterns weren't accepted by the parser previously.
Our goal is to avoid arbitrary patterns passed to functions without body through pat matchers becoming valid code (even if it causes warnings).
&IDENT and &&IDENT can be whitelisted in addition to mut IDENT, but this is annoying. I'm pretty sure we can make them errors now.

[nikomatsakis]

The code seems fine, the bigger question is the policy decision about moving to a hard error for at least some patterns. @petrochenkov what kind of crater runs do you have? Maybe we should do a pre-emptive run with this PR?

[petrochenkov]

@nikomatsakis

what kind of crater runs do you have?

From Jun 28, this year: #42894 (comment) #42894 (comment)

[durka]

Ah I see. So this is only one of the places that a #35203 warning can be generated and the other one is still a warning.

[durka]

Wait, I'm a little confused still, sorry. Is this the behavior I should expect after this PR?

macro_rules! m {
    ($pat: pat, $ty: ty) => {
        trait Tr {
            fn f($pat: $ty); // changed to have no body so #35203 is triggered
        }
    }
}

m!(x, u8); // OK
m!(mut x, u8); // warning #35203
m!(&x, &u8); // error
m!((x, y), (u8, u8)): // error

[nikomatsakis]

@durka that is what I expect, from reading the source anyway.

[nikomatsakis]

@durka In particular, this PR makes "complex" patterns like &x a hard error, regardless of whether they were written by hand or come from macro interpolation. I'm actually not 100% clear on the justification for that -- I have to go refresh my memory, I thought we decided on a "softer" deprecation stategy, though I'd love to see them go away.

[durka]

OK. I don't think it fully addresses #35203 (comment) then (for me that's a compelling use case), but it matches the previous plan.

[petrochenkov]

@durka

Is this the behavior I should expect after this PR?

Yes

[nikomatsakis]

Ah, I was misremembering. It's anonymous parameters that I was thinking of.

[durka]

Just for completeness, the reason it doesn't address that comment is even though you'll be able to write $pat: $ty in the macro expansion, you still can't write $pat:pat : $ty:ty in the pattern due to future proofing, so it at least halves the situations where you'd want to do it (limited to cases where the arg type comes from somewhere else).

[petrochenkov]

@durka

in the macro expansion, you still can't write $pat:pat : $ty:ty in the pattern due to future proofing

That's future proofing for type ascription in patterns

let (x: u8, y) = ...;

(and totally separate issue from this PR).

[durka]

I know the reasoning, I just wanted to point it out.

[nikomatsakis]

@rfcbot fcp merge

OK, well, I'd like to get formal approval on moving to a hard error, even for a limited number of cases. But I think it probably makes sense to go forward here. To summarize the situation:

• It is currently a future-compatibility warning to have complex patterns in functions without bodies. These don't make much logical sense regardless.
  • Examples:
    • trait Foo { fn bar(mut self); } -- the mut here is really a property of the implementation, which is not provided here.
    • trait Foo { fn bar((a, b): Self); }
• In part, these were banned because we did not check them at all (e.g., no error would be issued for using a (a, b) pattern even if the type of the argument was not a tuple.
• On a related note, macro interpolation of patterns does not work in this context trait Foo { fn bar($pat: u32) }.
• This PR:
  • Makes it a hard-error (no longer just warning) to use any pattern in such a context except for mut foo.
    • The latter is the only thing used in practice, according to prior crater runs.
      • @petrochenkov -- clarification, is it literally the only thing used in practice?
  • Enables interpolation, as long as the interpolated pattern fits the requirements.

[petrochenkov]

@rfcbot is on vacation

clarification, is it literally the only thing used in practice?

From 20 patterns_in_fns_without_body-related regressions found by crater in #42894 (comment), 19 are mut IDENT and 1 is &IDENT (glitter-0.1.1).

[nikomatsakis]

Sigh. No @rfcbot? OK, I will nominate for discussion at today's mtg.

[nikomatsakis]

@petrochenkov have we tried to patch glitter-0.1.1? That's the latest version of glitter, though the crate looks unmaintained I admit.

[nikomatsakis]

Discussed in @rust-lang/compiler meeting and decided to go forward with this PR.

[nikomatsakis]

@bors r+

[bors]

📌 Commit 10c0de3 has been approved by nikomatsakis

[bors]

⌛ Testing commit 10c0de365d9cc61547ba66f3ea938688757a95f7 with merge d550e9f20fa647e7d1698d44627d77f240f73136...

[bors]

💔 Test failed - status-appveyor

[kennytm]

Needs to change the error code.

tidy error: duplicate error code: 641
tidy error: C:\projects\rust\src\librustc_passes\diagnostics.rs:267:     E0641, // patterns aren't allowed in methods without bodies
tidy error: C:\projects\rust\src\librustc_typeck\diagnostics.rs:4746:     E0641, // cannot cast to/from a pointer with an unknown kind
some tidy checks failed

[petrochenkov]

@bors r=nikomatsakis

[bors]

📌 Commit f7b4b88 has been approved by nikomatsakis

[bors]

⌛ Testing commit f7b4b88 with merge b226793...

[petrochenkov]

@nikomatsakis

have we tried to patch glitter-0.1.1? That's the latest version of glitter, though the crate looks unmaintained I admit.

I've sent a PR for glitter (kylewlacy/glitter#3).

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing b226793 to master...

[nikomatsakis]

Marking for relnotes. This PR moves along the process of making argument patterns in functions without bodies a hard error (see #35203 for details). In particular, we now only support mut x patterns.