Skip to content

reject Reborrow impls on types with &'static mut fields - #165006

Open
paradoxicalguy wants to merge 1 commit into
rust-lang:mainfrom
paradoxicalguy:fix-reborrow-static-mut-ice
Open

paradoxicalguy wants to merge 1 commit into
rust-lang:mainfrom
paradoxicalguy:fix-reborrow-static-mut-ice

Conversation

@paradoxicalguy

@paradoxicalguy paradoxicalguy commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

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 rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 26, 2026
@rustbot

rustbot commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

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

@rustbot

This comment has been minimized.

@paradoxicalguy

Copy link
Copy Markdown
Member Author

oops

@paradoxicalguy
paradoxicalguy force-pushed the fix-reborrow-static-mut-ice branch from 566ba0a to 48aa1c8 Compare October 26, 2026 22:40
@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Jules-Bertholet

Copy link
Copy Markdown
Contributor

What about:

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

@paradoxicalguy
paradoxicalguy force-pushed the fix-reborrow-static-mut-ice branch from 48aa1c8 to a04e708 Compare October 27, 2026 09:22
@rustbot

rustbot commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Contributor

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

paradoxicalguy commented Oct 27, 2026 •

Copy link
Copy Markdown
Member Author

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

@paradoxicalguy
paradoxicalguy force-pushed the fix-reborrow-static-mut-ice branch from a04e708 to 4f03448 Compare October 27, 2026 21:03
@Jules-Bertholet

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member Author

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
paradoxicalguy force-pushed the fix-reborrow-static-mut-ice branch from 4f03448 to 25f3d21 Compare October 30, 2026 10:28
@paradoxicalguy
paradoxicalguy force-pushed the fix-reborrow-static-mut-ice branch from 25f3d21 to 80e4c80 Compare October 30, 2026 10:33
@paradoxicalguy

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

cc @fee1-dead

@rust-bors

rust-bors Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ICE]: Cannot implement Reborrow on a type containing a &'static mut T field

5 participants