Don't require unsafe for struct and array patterns against union fields

[Jules-Bertholet]

These patterns don't access the union directly, only their subpatterns do. So there is no need to require unsafe.

For example, the following now compiles:

union Foo {
    field: [u8; 3],
}

fn bar(foo: Foo) {
    match foo {
        Foo { field: [_, _, _], } => (),
    }
}

Also removes the unsafe requirement for the unstable guard patterns (#129967) in this position.

Reference PR: rust-lang/reference#2350

@rustbot label T-lang needs-fcp A-patterns F-guard_patterns I-lang-nominated

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 16 candidates

[folkertdev]

You should actually nominate for T-lang if you want them to look at this.

[traviscross]

Worth noting, for our discussion, that the example in the PR description compiles today, i.e., ahead of this PR landing:

union Foo {
    field: u8,
}

fn bar(foo: Foo) {
    match foo {
        Foo { field: _ } => (), // OK!
    }
}

What fails are things like this:

union Foo {
    field: (u8,),
}

fn bar(foo: Foo) {
    match foo {
        Foo { field: (..) } => (), // ERROR: Needs `unsafe`.
    }
}

[Jules-Bertholet]

Thanks for the correction, I edited the OP

[scottmcm]

Just checking: we still FakeRead/PlaceReference (I don't remember which it should be) the union when doing this, right?

Assuming we do, then I agree that allowing these categories of irrefutable patterns in union patterns makes sense 👍

[scottmcm]

Pondering: [..] is also an irrefutable pattern but isn't updated here (right?)

I don't know if it's possible, but could this whole match change to being about irrefutable pattern instead, or something? If we have to whack-a-mole a whole bunch of things here, that makes me less "oh yeah let's do it" than I was before, since I don't know why people would write this.

(Notably if you're using a pat_param from a macro it'd actually be easier for it to always be unsafe so you don't need to suppress the unneeded-unsafe if they pass something simple.)

Part of why we said that unsafeck is on THIR is that it's more of a lexical check than a flow-sensitive one, so being a bit more unsafe than strictly necessary is generally fine if it's something that the human description of the thing is something that people would say "it's unsafe to do that".

View changes since the review

[Jules-Bertholet]

Pondering: [..] is also an irrefutable pattern but isn't updated here (right?)

Yes it is. There's even a test.

could this whole match change to being about irrefutable pattern instead

No, irrefutability isn't sufficient. x is an irrefutable pattern but still needs to be unsafe; & _ probably should be as well. Nor is it even necessary; the unstable guard patterns are refutable, but shouldn't require unsafe.

What we care about is that the pattern does not perform a read/assert validity.

[tmandry]

We talked about this in today's lang meeting. This change seems fine individually, but along the lines of @scottmcm's comments above, I'd like to do it in a way that preserves (or improves) the overall consistency and simplicity of the language.

One way to evaluate that would be to review a reference PR for the change. @Jules-Bertholet would you be willing to draft one for us to review?

[Jules-Bertholet]

@tmandry Here you go: rust-lang/reference#2350

While drafting, I noticed one issue: this change makes #162213 slightly worse, because now whether a const pattern requires unsafe can depend on what should be private implementation details of the const item. (I don't think that issue should block this change, as the semver hazard exists either way.) Edit: this has been fixed

[theemathas]

This doesn't correctly handle guard patterns. Currently, it flags any patterns inside a guard pattern condition inside a union pattern as also accessing the union. This is incorrect. For example, the following code doesn't compile, even with this PR, but I think it should:

#![feature(guard_patterns)]
#![expect(incomplete_features)]

union MyUnion {
    thing: i32,
}

fn foo(x: MyUnion) {
    match x {
        MyUnion {
            thing: (_ if matches!(1, 1)),
        } => {}
    }
}

The in_union_destructure flag needs to reset to false when walking the guard pattern condition.

View changes since the review

[Jules-Bertholet]

@theemathas see the newly added test

[Jules-Bertholet]

Whoops, I forgot the important part… 🤦 fixed

[rustbot]

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred in match checking

cc @Nadrieril

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Jules-Bertholet]

I've addressed the semver hazard issue as it pertains to this PR. The Reference PR has also been updated.

[Nadrieril]

I'm confused by the logic here. Are you using has_rest/privacy to detect constant patterns? I'd prefer to add a thir pattern node that remembers patterns that were lowered from a constant (this keeps coming up and will be added by #155216 anyway), and use that to know whether we're inside a constant.

View changes since the review

[Nadrieril]

I'm generally confused by what has_rest has to do with this PR, given that it's equivalent to a bunch of wildcard patterns anyway.

[Jules-Bertholet]

Are you using has_rest/privacy to detect constant patterns?

Yes, specifically constant patterns whose equivalent expanded pattern could not have been written directly at the location the pattern is being used. If a struct has non-visible fields or is foreign non_exhaustive, then you need a rest pattern to do a direct pattern match; the absence of such a pattern (constants don't have rest patterns) means the pattern was expanded from a constant in another crate.

I was going for the smallest possible change; if you have a suggestion for a cleaner way to carry though this information, that's fine, will gladly do it your way.