[paradoxicalguy]

fixes #164933

the Reborrow impl check accepted every &mut field, so a type with a &'static mut T field passed coherence and then hit a bug! in borrowck. this makes &'static mut fall through to the Copy check, so the impl is rejected with a normal error instead of an ice.
adds a ui test using the repro from the issue.

edit:
also updates invalid-coerce-shared-static-mut-field: its Reborrow impl is now rejected at the impl, so the test gains an E0277.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 77 candidates
• Random selection from 22 candidates

[paradoxicalguy]

oops

[Jules-Bertholet]

What about:

struct Thing<'a: 'static> {
    field: &'a mut u8,
}

[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.

[paradoxicalguy]

the check now looks at the declared field type rather than the substituted one, so Wrap<'a, &'static mut i32> (reborrow-generic-mut-field-aliasing) stays valid. added a test for the 'a: 'static case; it gives a normal borrowck error, no ice.

[Jules-Bertholet]

It gives a borrowcheck error when you actually try to use the Reborrow impl, but ideally we would error on the impl itself. This becomes more important once we add support for generic T: Reborrow.

[paradoxicalguy]

:O, yes
i'll extend the test

pushed: the impl is now rejected for 'a: 'static bounds too, with an E0277 on the field (test: reborrow-outlives-static-mut-field). limitation: it looks at the impl's where-clauses directly, so transitive bounds like 'a: 'b, 'b: 'static aren't caught yet. happy to follow up with a full region-resolution check if you want that in this pr.

[Jules-Bertholet]

In general, I don't think we should keep adding ad-hoc checks here without a solid plan for the end-state. This is an unstable feature, it's fine if there are rough edges.

[paradoxicalguy]

yeah i think i went nuts; been extending the check around individual cases without establishing the general rule first.

+would you prefer keeping this pr scoped to the original literal &'static mut ICE with a minimal check, or should we first establish how the coherence check and borrowck should handle effectively-static regions more generally?

i'd rather get the invariant right than keep extending an incomplete region-bound scan

[paradoxicalguy]

pushed a simpler version of the fix. it now only checks for literal &'static mut fields and removes the caller_bounds() scan, since that was getting too ad-hoc.

i also tested the 'a: 'static case separately. it gives normal borrowck errors (E0597/E0499) instead of an ICE, though the impl itself is still accepted.

all 125 reborrow UI tests pass, and tidy is clean.

keeping this scoped to the original ICE for now rather than trying to handle every lifetime bound here.

[paradoxicalguy]

cc @fee1-dead

[fee1-dead]

Sorry, was really swamped IRL..

cc @aapoalas

@rustbot reroll

[paradoxicalguy]

np, i also have to solve merge conflicts :D

[aapoalas]

@rustbot r? @me

[aapoalas]

question: After the merge of #163284 I'm wondering if this would get the same \x` does not live long enougherror without the main changes of this PR implemented? I believe we should already have tests showing that eg.Thing<'a>(PhantomData<&'a ()>, &'static mut u8)can implement Reborrow and it will even work if the end result is that all the Reborrows are always equal to just Moves and we start with a&'static mut u8` in the first place.

View changes since the review